From 15576d7b3f4f139e0843138bf61d358342b12b04 Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Sat, 11 Apr 2026 16:59:40 +0200 Subject: [PATCH 1/8] feat: marketplace-based version management (Phase 1) Add marketplace version schema, semver range resolution, and version-aware install/view/outdated flows for Issue #514. - Add VersionEntry model and versions[] field to MarketplacePlugin - Create semver range resolver (^, ~, >=, exact, compound ranges) - Version-aware install: resolve marketplace versions with #specifier syntax - Version-aware view: apm view plugin@marketplace versions shows version table - Version-aware outdated: marketplace deps checked against marketplace versions - Add version_spec field to LockedDependency for lockfile persistence - 108 new tests across 5 test files, all 3909 tests passing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/commands/install.py | 5 +- src/apm_cli/commands/outdated.py | 162 +++++- src/apm_cli/commands/view.py | 129 +++++ src/apm_cli/deps/lockfile.py | 4 + src/apm_cli/marketplace/models.py | 32 ++ src/apm_cli/marketplace/resolver.py | 64 ++- src/apm_cli/marketplace/version_resolver.py | 256 ++++++++++ .../test_marketplace_install_integration.py | 2 +- .../marketplace/test_marketplace_models.py | 198 ++++++++ .../marketplace/test_marketplace_resolver.py | 6 +- .../marketplace/test_versioned_resolver.py | 387 +++++++++++++++ tests/unit/test_outdated_marketplace.py | 464 ++++++++++++++++++ tests/unit/test_version_resolver.py | 322 ++++++++++++ tests/unit/test_view_versions.py | 396 +++++++++++++++ 14 files changed, 2395 insertions(+), 32 deletions(-) create mode 100644 src/apm_cli/marketplace/version_resolver.py create mode 100644 tests/unit/marketplace/test_versioned_resolver.py create mode 100644 tests/unit/test_outdated_marketplace.py create mode 100644 tests/unit/test_version_resolver.py create mode 100644 tests/unit/test_view_versions.py diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 4e39a810..92098c6b 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -150,7 +150,7 @@ def _validate_and_add_packages_to_apm_yml(packages, dry_run=False, dev=False, lo mkt_ref = None if mkt_ref is not None: - plugin_name, marketplace_name = mkt_ref + plugin_name, marketplace_name, version_spec = mkt_ref try: if logger: logger.verbose_detail( @@ -159,6 +159,7 @@ def _validate_and_add_packages_to_apm_yml(packages, dry_run=False, dev=False, lo canonical_str, resolved_plugin = resolve_marketplace_plugin( plugin_name, marketplace_name, + version_spec=version_spec, auth_resolver=auth_resolver, ) if logger: @@ -168,6 +169,7 @@ def _validate_and_add_packages_to_apm_yml(packages, dry_run=False, dev=False, lo marketplace_provenance = { "discovered_via": marketplace_name, "marketplace_plugin_name": plugin_name, + "version_spec": version_spec, } package = canonical_str except Exception as mkt_err: @@ -2621,6 +2623,7 @@ def _collect_descendants(node, visited=None): if dep_key in lockfile.dependencies: lockfile.dependencies[dep_key].discovered_via = prov.get("discovered_via") lockfile.dependencies[dep_key].marketplace_plugin_name = prov.get("marketplace_plugin_name") + lockfile.dependencies[dep_key].version_spec = prov.get("version_spec") # Selectively merge entries from the existing lockfile: # - For partial installs (only_packages): preserve all old entries # (sequential install — only the specified package was processed). diff --git a/src/apm_cli/commands/outdated.py b/src/apm_cli/commands/outdated.py index 2bef64cc..bd7f8089 100644 --- a/src/apm_cli/commands/outdated.py +++ b/src/apm_cli/commands/outdated.py @@ -2,13 +2,16 @@ Compares locked dependency commit SHAs against remote tip SHAs. For tag-pinned deps, also shows the latest available semver tag. +For marketplace-sourced deps, checks available versions in the marketplace. """ +import logging import re import sys import click +logger = logging.getLogger(__name__) TAG_RE = re.compile(r"^v?\d+\.\d+\.\d+") @@ -53,12 +56,119 @@ def _find_remote_tip(ref_name, remote_refs): return None +def _check_marketplace_versions(dep, verbose): + """Check a marketplace-sourced dep against its marketplace versions. + + Returns a result tuple ``(package, current, latest, status, extra, source)`` + or ``None`` when the check cannot be performed (caller should fall through + to the git-based check). + """ + if not dep.discovered_via or not dep.marketplace_plugin_name: + return None + + current_ver = dep.version or "" + if not current_ver: + return None + + try: + from ..marketplace.client import fetch_or_cache + from ..marketplace.errors import MarketplaceError + from ..marketplace.registry import get_marketplace_by_name + from ..marketplace.version_resolver import ( + _expand_specifier, + _parse_semver, + _version_matches, + ) + except ImportError: + return None + + source_label = f"marketplace: {dep.discovered_via}" + + try: + source_obj = get_marketplace_by_name(dep.discovered_via) + except MarketplaceError: + logger.warning( + "Marketplace '%s' not found; falling back to git check for '%s'", + dep.discovered_via, + dep.marketplace_plugin_name, + ) + return None + + try: + manifest = fetch_or_cache(source_obj) + except MarketplaceError: + logger.warning( + "Failed to fetch marketplace '%s'; " + "falling back to git check for '%s'", + dep.discovered_via, + dep.marketplace_plugin_name, + ) + return None + + plugin = manifest.find_plugin(dep.marketplace_plugin_name) + if not plugin or not plugin.versions: + return None # No versions list -> fall through to git + + # Parse current version + try: + current_parsed = _parse_semver(current_ver) + except ValueError: + return None + + # Find highest version in marketplace + best_parsed = None + best_entry = None + for entry in plugin.versions: + try: + parsed = _parse_semver(entry.version) + except ValueError: + continue + if best_parsed is None or parsed > best_parsed: + best_parsed = parsed + best_entry = entry + + if best_entry is None or best_parsed is None: + return None # No valid semver versions found + + package_name = f"{dep.marketplace_plugin_name}@{dep.discovered_via}" + + if best_parsed > current_parsed: + latest_display = best_entry.version + + # Check version_spec if available (field added by parallel task) + version_spec = getattr(dep, "version_spec", None) + if version_spec: + constraints = _expand_specifier(version_spec) + if constraints and not _version_matches(best_parsed, constraints): + latest_display = ( + f"{best_entry.version} (outside range {version_spec})" + ) + + extra = [e.version for e in plugin.versions[:10]] if verbose else [] + return ( + package_name, current_ver, latest_display, + "outdated", extra, source_label, + ) + + return ( + package_name, current_ver, best_entry.version, + "up-to-date", [], source_label, + ) + + def _check_one_dep(dep, downloader, verbose): """Check a single dependency against remote refs. - Returns a result tuple: (package_name, current, latest, status, extra_tags) + Returns a result tuple: + ``(package_name, current, latest, status, extra_tags, source)`` + This function is safe to call from a thread pool. """ + # Try marketplace-based check first for marketplace-sourced deps + marketplace_result = _check_marketplace_versions(dep, verbose) + if marketplace_result is not None: + return marketplace_result + from ..models.dependency.reference import DependencyReference from ..models.dependency.types import GitReferenceType from ..utils.version_checker import is_newer_version @@ -73,20 +183,20 @@ def _check_one_dep(dep, downloader, verbose): full_url = f"{dep.host}/{dep.repo_url}" if dep.host else dep.repo_url dep_ref = DependencyReference.parse(full_url) except Exception: - return (package_name, current_ref or "(none)", "-", "unknown", []) + return (package_name, current_ref or "(none)", "-", "unknown", [], "") # Fetch remote refs try: remote_refs = downloader.list_remote_refs(dep_ref) except Exception: - return (package_name, current_ref or "(none)", "-", "unknown", []) + return (package_name, current_ref or "(none)", "-", "unknown", [], "") is_tag = _is_tag_ref(current_ref) if is_tag: tag_refs = [r for r in remote_refs if r.ref_type == GitReferenceType.TAG] if not tag_refs: - return (package_name, current_ref, "-", "unknown", []) + return (package_name, current_ref, "-", "unknown", [], "git tags") latest_tag = tag_refs[0].name current_ver = _strip_v(current_ref) @@ -94,21 +204,21 @@ def _check_one_dep(dep, downloader, verbose): if is_newer_version(current_ver, latest_ver): extra = [r.name for r in tag_refs[:10]] if verbose else [] - return (package_name, current_ref, latest_tag, "outdated", extra) + return (package_name, current_ref, latest_tag, "outdated", extra, "git tags") else: - return (package_name, current_ref, latest_tag, "up-to-date", []) + return (package_name, current_ref, latest_tag, "up-to-date", [], "git tags") else: remote_tip_sha = _find_remote_tip(current_ref, remote_refs) if not remote_tip_sha: - return (package_name, current_ref or "(none)", "-", "unknown", []) + return (package_name, current_ref or "(none)", "-", "unknown", [], "git branch") display_ref = current_ref or "(default)" if locked_sha and locked_sha != remote_tip_sha: latest_display = remote_tip_sha[:8] - return (package_name, display_ref, latest_display, "outdated", []) + return (package_name, display_ref, latest_display, "outdated", [], "git branch") else: - return (package_name, display_ref, remote_tip_sha[:8], "up-to-date", []) + return (package_name, display_ref, remote_tip_sha[:8], "up-to-date", [], "git branch") @click.command(name="outdated") @@ -185,8 +295,8 @@ def outdated(global_, verbose, parallel_checks): return # Check if everything is up-to-date - has_outdated = any(status == "outdated" for _, _, _, status, _ in rows) - has_unknown = any(status == "unknown" for _, _, _, status, _ in rows) + has_outdated = any(status == "outdated" for _, _, _, status, _, _ in rows) + has_unknown = any(status == "unknown" for _, _, _, status, _, _ in rows) if not has_outdated and not has_unknown: logger.success("All dependencies are up-to-date") @@ -210,6 +320,7 @@ def outdated(global_, verbose, parallel_checks): table.add_column("Package", style="white", min_width=20) table.add_column("Current", style="white", min_width=10) table.add_column("Latest", style="white", min_width=10) + table.add_column("Source", style="dim", min_width=14) table.add_column("Status", min_width=12) status_styles = { @@ -218,27 +329,36 @@ def outdated(global_, verbose, parallel_checks): "unknown": "dim", } - for package, current, latest, status, extra_tags in rows: + for package, current, latest, status, extra_tags, source in rows: style = status_styles.get(status, "white") - table.add_row(package, current, latest, f"[{style}]{status}[/{style}]") + table.add_row( + package, current, latest, source, + f"[{style}]{status}[/{style}]", + ) if verbose and extra_tags: tags_str = ", ".join(extra_tags) - table.add_row("", "", f"[dim]tags: {tags_str}[/dim]", "") + table.add_row("", "", f"[dim]tags: {tags_str}[/dim]", "", "") console.print(table) except (ImportError, Exception): # Fallback: plain text output - click.echo("Package Current Latest Status") - click.echo("-" * 65) - for package, current, latest, status, extra_tags in rows: - click.echo(f"{package:<24}{current:<13}{latest:<13}{status}") + click.echo( + f"{'Package':<24}{'Current':<13}{'Latest':<13}" + f"{'Source':<20}{'Status'}" + ) + click.echo("-" * 82) + for package, current, latest, status, extra_tags, source in rows: + click.echo( + f"{package:<24}{current:<13}{latest:<13}" + f"{source:<20}{status}" + ) if verbose and extra_tags: click.echo(f"{'':24}tags: {', '.join(extra_tags)}") # Summary - outdated_count = sum(1 for _, _, _, s, _ in rows if s == "outdated") + outdated_count = sum(1 for _, _, _, s, _, _ in rows if s == "outdated") if outdated_count: logger.warning(f"{outdated_count} outdated " f"{'dependency' if outdated_count == 1 else 'dependencies'} found") @@ -323,7 +443,7 @@ def _check_parallel(checkable, downloader, verbose, max_workers, result = fut.result() except Exception: pkg = dep.get_unique_key() - result = (pkg, "(none)", "-", "unknown", []) + result = (pkg, "(none)", "-", "unknown", [], "") results[dep.get_unique_key()] = result progress.update(task_id, visible=False) progress.advance(overall_id) @@ -350,7 +470,7 @@ def _check_parallel_plain(checkable, downloader, verbose, max_workers): result = fut.result() except Exception: pkg = dep.get_unique_key() - result = (pkg, "(none)", "-", "unknown", []) + result = (pkg, "(none)", "-", "unknown", [], "") results[dep.get_unique_key()] = result return [results[dep.get_unique_key()] for dep in checkable diff --git a/src/apm_cli/commands/view.py b/src/apm_cli/commands/view.py index e8c1b00c..0f4f1c05 100644 --- a/src/apm_cli/commands/view.py +++ b/src/apm_cli/commands/view.py @@ -243,6 +243,121 @@ def display_package_info( sys.exit(1) +def _display_marketplace_versions( + plugin_name: str, + marketplace_name: str, + logger: CommandLogger, +) -> None: + """Display version history for a marketplace plugin. + + Fetches the marketplace manifest, finds the plugin, and renders its + ``versions[]`` array as a Rich table (with plain-text fallback). + """ + from ..marketplace.errors import MarketplaceFetchError, PluginNotFoundError + from ..marketplace.models import MarketplaceSource + from ..marketplace.registry import get_marketplace_by_name + from ..marketplace.client import fetch_or_cache + from ..marketplace.version_resolver import _parse_semver, _SEMVER_RE + + # -- Fetch marketplace & plugin -- + try: + source: MarketplaceSource = get_marketplace_by_name(marketplace_name) + except Exception as exc: + # MarketplaceNotFoundError carries actionable guidance + logger.error(str(exc)) + sys.exit(1) + + try: + manifest = fetch_or_cache(source) + except MarketplaceFetchError as exc: + logger.error(str(exc)) + logger.progress("Check your network connection and try again.") + sys.exit(1) + + plugin = manifest.find_plugin(plugin_name) + if plugin is None: + from ..marketplace.errors import PluginNotFoundError as _PNF + + logger.error(str(_PNF(plugin_name, marketplace_name))) + sys.exit(1) + + versions = plugin.versions + if not versions: + logger.progress( + f"No version history available for '{plugin_name}'. " + f"Using single-ref source." + ) + return + + # -- Sort by semver descending; non-semver entries go to the end -- + semver_entries = [] + non_semver_entries = [] + for entry in versions: + if _SEMVER_RE.match(entry.version.strip()): + try: + parsed = _parse_semver(entry.version) + semver_entries.append((parsed, entry)) + except ValueError: + non_semver_entries.append(entry) + else: + non_semver_entries.append(entry) + + semver_entries.sort(key=lambda c: c[0], reverse=True) + sorted_versions = [e for _, e in semver_entries] + non_semver_entries + + # Determine the "latest" version (only if semver-sorted entries exist) + latest_version = semver_entries[0][1].version if semver_entries else None + + # -- Render -- + title = ( + f"Available versions: {plugin_name} " + f"(marketplace: {marketplace_name})" + ) + try: + from rich.console import Console + from rich.table import Table + + console = Console() + table = Table( + title=title, + show_header=True, + header_style="bold cyan", + ) + table.add_column("Version", style="bold white") + table.add_column("Ref", style="dim white") + table.add_column("Status", style="yellow") + + for entry in sorted_versions: + status = "latest" if entry.version == latest_version else "" + table.add_row(entry.version, entry.ref, status) + + console.print(table) + click.echo("") + click.echo(f" Install: apm install {plugin_name}@{marketplace_name}") + click.echo( + f" Pin: apm install {plugin_name}@{marketplace_name}" + f"#^{sorted_versions[0].version}" + ) + + except ImportError: + # Plain-text fallback + click.echo(title) + click.echo("-" * 60) + click.echo(f"{'Version':<20} {'Ref':<30} {'Status':<10}") + click.echo("-" * 60) + for entry in sorted_versions: + status = "latest" if entry.version == latest_version else "" + click.echo( + f"{entry.version:<20} {entry.ref:<30} {status:<10}" + ) + click.echo("") + click.echo(f" Install: apm install {plugin_name}@{marketplace_name}") + click.echo( + f" Pin: apm install {plugin_name}@{marketplace_name}" + f"#^{sorted_versions[0].version}" + ) + + def display_versions(package: str, logger: CommandLogger) -> None: """Query and display available remote versions (tags/branches). @@ -251,7 +366,21 @@ def display_versions(package: str, logger: CommandLogger) -> None: ``DependencyReference``, queries remote refs via ``GitHubPackageDownloader.list_remote_refs``, and renders the result as a Rich table (with a plain-text fallback). + + When *package* matches the ``NAME@MARKETPLACE`` pattern, the + marketplace manifest is fetched instead and the plugin's version + history is displayed. """ + # -- Marketplace path: NAME@MARKETPLACE -- + from ..marketplace.resolver import parse_marketplace_ref + + marketplace_ref = parse_marketplace_ref(package) + if marketplace_ref is not None: + plugin_name, marketplace_name, _version_spec = marketplace_ref + _display_marketplace_versions(plugin_name, marketplace_name, logger) + return + + # -- Git-based path (unchanged) -- try: dep_ref = DependencyReference.parse(package) except ValueError as exc: diff --git a/src/apm_cli/deps/lockfile.py b/src/apm_cli/deps/lockfile.py index e4782280..ac4bb98b 100644 --- a/src/apm_cli/deps/lockfile.py +++ b/src/apm_cli/deps/lockfile.py @@ -38,6 +38,7 @@ class LockedDependency: is_dev: bool = False # True for devDependencies discovered_via: Optional[str] = None # Marketplace name (provenance) marketplace_plugin_name: Optional[str] = None # Plugin name in marketplace + version_spec: Optional[str] = None # Original user semver range, e.g., "^2.0.0" def get_unique_key(self) -> str: """Returns unique key for this dependency.""" @@ -84,6 +85,8 @@ def to_dict(self) -> Dict[str, Any]: result["discovered_via"] = self.discovered_via if self.marketplace_plugin_name: result["marketplace_plugin_name"] = self.marketplace_plugin_name + if self.version_spec: + result["version_spec"] = self.version_spec return result @classmethod @@ -122,6 +125,7 @@ def from_dict(cls, data: Dict[str, Any]) -> "LockedDependency": is_dev=data.get("is_dev", False), discovered_via=data.get("discovered_via"), marketplace_plugin_name=data.get("marketplace_plugin_name"), + version_spec=data.get("version_spec"), ) @classmethod diff --git a/src/apm_cli/marketplace/models.py b/src/apm_cli/marketplace/models.py index 18a4484e..c807861d 100644 --- a/src/apm_cli/marketplace/models.py +++ b/src/apm_cli/marketplace/models.py @@ -53,6 +53,14 @@ def from_dict(cls, data: Dict[str, Any]) -> "MarketplaceSource": ) +@dataclass(frozen=True) +class VersionEntry: + """A single published version of a marketplace plugin.""" + + version: str # Semver string, e.g., "2.1.0" + ref: str # Git ref (SHA, tag, or branch name) + + @dataclass(frozen=True) class MarketplacePlugin: """A single plugin entry inside a marketplace manifest.""" @@ -62,6 +70,7 @@ class MarketplacePlugin: description: str = "" version: str = "" tags: Tuple[str, ...] = () + versions: Tuple[VersionEntry, ...] = () # Published version history source_marketplace: str = "" # Populated during resolution def matches_query(self, query: str) -> bool: @@ -121,6 +130,28 @@ def _parse_plugin_entry( raw_tags = entry.get("tags", []) tags = tuple(raw_tags) if isinstance(raw_tags, list) else () + # Parse optional versions array + versions: Tuple[VersionEntry, ...] = () + raw_versions = entry.get("versions", []) + if isinstance(raw_versions, list): + parsed: List[VersionEntry] = [] + for v_entry in raw_versions: + if not isinstance(v_entry, dict): + logger.debug( + "Skipping non-dict version entry in plugin '%s'", name + ) + continue + v_ver = v_entry.get("version", "") + v_ref = v_entry.get("ref", "") + if not v_ver or not v_ref: + logger.debug( + "Skipping version entry missing version/ref in plugin '%s'", + name, + ) + continue + parsed.append(VersionEntry(version=str(v_ver), ref=str(v_ref))) + versions = tuple(parsed) + # Determine source -- Copilot uses "repository", Claude uses "source" source: Any = None @@ -171,6 +202,7 @@ def _parse_plugin_entry( description=description, version=version, tags=tags, + versions=versions, source_marketplace=source_name, ) diff --git a/src/apm_cli/marketplace/resolver.py b/src/apm_cli/marketplace/resolver.py index 6c84fe7c..03b1cf12 100644 --- a/src/apm_cli/marketplace/resolver.py +++ b/src/apm_cli/marketplace/resolver.py @@ -20,23 +20,34 @@ logger = logging.getLogger(__name__) -_MARKETPLACE_RE = re.compile(r"^([a-zA-Z0-9._-]+)@([a-zA-Z0-9._-]+)$") +_MARKETPLACE_RE = re.compile( + r"^([a-zA-Z0-9._-]+)@([a-zA-Z0-9._-]+)(?:#(.+))?$" +) -def parse_marketplace_ref(specifier: str) -> Optional[Tuple[str, str]]: - """Parse a ``NAME@MARKETPLACE`` specifier. +def parse_marketplace_ref( + specifier: str, +) -> Optional[Tuple[str, str, Optional[str]]]: + """Parse a ``NAME@MARKETPLACE[#version_spec]`` specifier. + + The optional ``#version_spec`` suffix carries a semver range + (e.g. ``^2.0.0``) or a raw git ref (e.g. ``main``). Returns: - ``(plugin_name, marketplace_name)`` if the specifier matches, - or ``None`` if it does not look like a marketplace ref. + ``(plugin_name, marketplace_name, version_spec_or_none)`` if the + specifier matches, or ``None`` if it does not look like a + marketplace ref. """ s = specifier.strip() - # Quick rejection: slashes and colons belong to other formats - if "/" in s or ":" in s: + # Quick rejection: slashes and colons *before* the fragment belong to + # other formats. Split on ``#`` first so that refs with slashes + # (e.g. ``feature/branch``) don't cause a false rejection. + head = s.split("#", 1)[0] + if "/" in head or ":" in head: return None match = _MARKETPLACE_RE.match(s) if match: - return (match.group(1), match.group(2)) + return (match.group(1), match.group(2), match.group(3)) return None @@ -209,13 +220,21 @@ def resolve_marketplace_plugin( plugin_name: str, marketplace_name: str, *, + version_spec: Optional[str] = None, auth_resolver: Optional[object] = None, ) -> Tuple[str, MarketplacePlugin]: """Resolve a marketplace plugin reference to a canonical string. + When the plugin declares ``versions`` entries and a *version_spec* is + given (or ``None`` for latest), the version resolver selects the best + match and the returned canonical string carries the matching ref. + Args: plugin_name: Plugin name within the marketplace. marketplace_name: Registered marketplace name. + version_spec: Optional semver range (e.g. ``"^2.0.0"``) or raw + git ref. ``None`` selects the latest published version when + the plugin has ``versions``. auth_resolver: Optional ``AuthResolver`` instance. Returns: @@ -241,6 +260,35 @@ def resolve_marketplace_plugin( plugin_root=manifest.plugin_root, ) + # ---- Version-aware ref override ---- + if plugin.versions: + from .version_resolver import is_version_specifier, resolve_version + + if version_spec and not is_version_specifier(version_spec): + # Treat as a raw git ref -- override whatever ref came from + # the source field. + base = canonical.split("#", 1)[0] + canonical = f"{base}#{version_spec}" + logger.debug( + "Using raw git ref '%s' for %s@%s", + version_spec, + plugin_name, + marketplace_name, + ) + else: + # Resolve against the published versions list. + # version_spec=None -> latest (highest semver). + entry = resolve_version(version_spec, plugin.versions) + base = canonical.split("#", 1)[0] + canonical = f"{base}#{entry.ref}" + logger.debug( + "Resolved version %s (%s) for %s@%s", + entry.version, + entry.ref, + plugin_name, + marketplace_name, + ) + logger.debug( "Resolved %s@%s -> %s", plugin_name, diff --git a/src/apm_cli/marketplace/version_resolver.py b/src/apm_cli/marketplace/version_resolver.py new file mode 100644 index 00000000..c4ac8f01 --- /dev/null +++ b/src/apm_cli/marketplace/version_resolver.py @@ -0,0 +1,256 @@ +"""Semver range resolution for marketplace plugin versions. + +Resolves a user-provided version specifier against a list of available +VersionEntry objects from a marketplace plugin definition. +""" + +import logging +import re +from typing import List, Optional, Sequence, Tuple + +from .models import VersionEntry + +logger = logging.getLogger(__name__) + +# Strict semver pattern: X.Y.Z with integer components only. +# Pre-release and build metadata are intentionally unsupported. +_SEMVER_RE = re.compile(r"^(\d+)\.(\d+)\.(\d+)$") + +# Regex for quick classification of version specifiers. +# Matches: "X.Y.Z", "^X.Y.Z", "~X.Y.Z", ">=X.Y.Z", ">X.Y.Z", etc. +_SPECIFIER_RE = re.compile( + r"^\s*" + r"(?:" + r"[~^]?\d+\.\d+\.\d+" # Optional ^ or ~ prefix, then X.Y.Z + r"|[><=!]+\s*\d+\.\d+\.\d+" # Comparison operator then X.Y.Z + r")" + r"\s*$" +) + +# Comparison operators at the start of a constraint clause. +_OP_RE = re.compile(r"^(>=|<=|!=|>|<|==)\s*(.+)$") + +# Type alias for parsed version tuples. +SemverTuple = Tuple[int, int, int] + +# Type alias for a single constraint: (operator, version_tuple). +Constraint = Tuple[str, SemverTuple] + + +def _parse_semver(version_str: str) -> SemverTuple: + """Parse a strict ``X.Y.Z`` version string into an integer tuple. + + Args: + version_str: Version string to parse (e.g. ``"2.1.0"``). + + Returns: + Tuple of ``(major, minor, patch)``. + + Raises: + ValueError: If the string is not valid ``X.Y.Z`` semver. + """ + s = version_str.strip() + m = _SEMVER_RE.match(s) + if not m: + raise ValueError( + f"Invalid semver version: '{version_str}'. " + f"Expected format: X.Y.Z (e.g. '2.1.0')" + ) + return (int(m.group(1)), int(m.group(2)), int(m.group(3))) + + +def _version_matches(version: SemverTuple, constraints: List[Constraint]) -> bool: + """Check whether *version* satisfies every constraint in the list. + + Constraints are ``(operator, target_tuple)`` pairs where operator is one + of ``==``, ``!=``, ``>=``, ``>``, ``<=``, ``<``. + """ + for op, target in constraints: + if op == "==" and not (version == target): + return False + elif op == "!=" and not (version != target): + return False + elif op == ">=" and not (version >= target): + return False + elif op == ">" and not (version > target): + return False + elif op == "<=" and not (version <= target): + return False + elif op == "<" and not (version < target): + return False + return True + + +def _expand_caret(ver: SemverTuple) -> List[Constraint]: + """Expand caret (``^``) specifier to constraint list. + + Caret means "compatible with": the upper bound increments the leftmost + non-zero component. + + - ``^1.2.3`` -> ``>=1.2.3, <2.0.0`` + - ``^0.5.0`` -> ``>=0.5.0, <0.6.0`` + - ``^0.0.3`` -> ``>=0.0.3, <0.0.4`` + - ``^0.0.0`` -> ``>=0.0.0, <0.0.1`` + """ + major, minor, patch = ver + if major != 0: + upper = (major + 1, 0, 0) + elif minor != 0: + upper = (0, minor + 1, 0) + else: + upper = (0, 0, patch + 1) + return [(">=", ver), ("<", upper)] + + +def _expand_tilde(ver: SemverTuple) -> List[Constraint]: + """Expand tilde (``~``) specifier to constraint list. + + Tilde means "patch-level changes only": + ``~X.Y.Z`` -> ``>=X.Y.Z, =", ver), ("<", (major, minor + 1, 0))] + + +def _parse_single_clause(clause: str) -> List[Constraint]: + """Parse one clause of a version specifier into constraints. + + Supported forms: + - ``"2.1.0"`` (exact match) + - ``"^2.0.0"`` (caret / compatible) + - ``"~2.1.0"`` (tilde / patch-level) + - ``">=1.5.0"``, ``">1.0.0"``, ``"<3.0.0"``, ``"<=2.0.0"``, ``"!=1.0.0"`` + """ + s = clause.strip() + if not s: + raise ValueError("Empty version clause") + + # Caret prefix + if s.startswith("^"): + ver = _parse_semver(s[1:]) + return _expand_caret(ver) + + # Tilde prefix + if s.startswith("~"): + ver = _parse_semver(s[1:]) + return _expand_tilde(ver) + + # Comparison operator + m = _OP_RE.match(s) + if m: + op = m.group(1) + ver = _parse_semver(m.group(2)) + return [(op, ver)] + + # Bare version -> exact match + ver = _parse_semver(s) + return [("==", ver)] + + +def _expand_specifier(specifier: Optional[str]) -> List[Constraint]: + """Expand a full specifier string into a flat list of constraints. + + Supports comma-separated compound specifiers like ``">=1.0.0,<3.0.0"``. + Returns an empty list when *specifier* is ``None`` or empty (meaning + "latest / no constraint"). + """ + if not specifier or not specifier.strip(): + return [] + + constraints: List[Constraint] = [] + for clause in specifier.split(","): + constraints.extend(_parse_single_clause(clause)) + return constraints + + +# --------------------------------------------------------------------------- +# Public API +# --------------------------------------------------------------------------- + + +def resolve_version( + specifier: Optional[str], + versions: Sequence["VersionEntry"], +) -> "VersionEntry": + """Resolve a version specifier against available versions. + + Args: + specifier: Version specifier string, or ``None`` for latest. + Supported formats: + + - ``None`` or empty -> latest (highest semver) + - ``"2.1.0"`` -> exact match + - ``"^2.0.0"`` -> compatible (``>=2.0.0, <3.0.0``) + - ``"~2.1.0"`` -> patch-level (``>=2.1.0, <2.2.0``) + - ``">=1.5.0"`` -> minimum version + - ``">=1.0.0,<3.0.0"`` -> compound range + + versions: Available versions from marketplace plugin. + + Returns: + The best matching ``VersionEntry`` (highest version satisfying + the constraint). + + Raises: + ValueError: If no version matches the specifier, or the + versions list is empty. + """ + if not versions: + raise ValueError("No versions available to resolve against") + + constraints = _expand_specifier(specifier) + + # Build candidates: skip entries whose version string is not valid semver. + candidates: List[Tuple[SemverTuple, "VersionEntry"]] = [] + for entry in versions: + try: + parsed = _parse_semver(entry.version) + except ValueError: + logger.debug( + "Skipping version entry with invalid semver: '%s'", + entry.version, + ) + continue + if _version_matches(parsed, constraints): + candidates.append((parsed, entry)) + + if not candidates: + spec_desc = specifier if specifier else "latest" + available = ", ".join( + e.version for e in versions if _SEMVER_RE.match(e.version.strip()) + ) + raise ValueError( + f"No version matches specifier '{spec_desc}'. " + f"Available versions: {available or '(none valid)'}" + ) + + # Sort by semver tuple descending and return the highest match. + candidates.sort(key=lambda c: c[0], reverse=True) + return candidates[0][1] + + +def is_version_specifier(value: str) -> bool: + """Check if a string looks like a semver version specifier. + + Returns ``True`` for values that should be interpreted as version + constraints (e.g. ``"2.1.0"``, ``"^2.0.0"``, ``"~2.1.0"``, + ``">=1.5.0"``). + + Returns ``False`` for values that look like git refs (e.g. + ``"main"``, ``"abc123def"``, ``"feature/branch"``). + + This is a heuristic used for routing: when a user passes a string + that could be either a version or a git ref, this function decides + which interpretation to use. + """ + if not value or not value.strip(): + return False + + # Check each comma-separated clause independently. + for clause in value.split(","): + clause = clause.strip() + if not clause: + return False + if not _SPECIFIER_RE.match(clause): + return False + return True diff --git a/tests/unit/marketplace/test_marketplace_install_integration.py b/tests/unit/marketplace/test_marketplace_install_integration.py index 6399e131..9cef01c7 100644 --- a/tests/unit/marketplace/test_marketplace_install_integration.py +++ b/tests/unit/marketplace/test_marketplace_install_integration.py @@ -13,7 +13,7 @@ class TestInstallMarketplacePreParse: def test_marketplace_ref_detected(self): """NAME@MARKETPLACE triggers marketplace resolution.""" result = parse_marketplace_ref("security-checks@acme-tools") - assert result == ("security-checks", "acme-tools") + assert result == ("security-checks", "acme-tools", None) def test_owner_repo_not_intercepted(self): """owner/repo should NOT be intercepted.""" diff --git a/tests/unit/marketplace/test_marketplace_models.py b/tests/unit/marketplace/test_marketplace_models.py index 2fc708ce..f218b488 100644 --- a/tests/unit/marketplace/test_marketplace_models.py +++ b/tests/unit/marketplace/test_marketplace_models.py @@ -6,6 +6,7 @@ MarketplaceManifest, MarketplacePlugin, MarketplaceSource, + VersionEntry, parse_marketplace_json, ) @@ -369,3 +370,200 @@ def test_plugin_root_missing_key(self): data = {"name": "Test", "metadata": {"version": "1.0"}, "plugins": []} manifest = parse_marketplace_json(data) assert manifest.plugin_root == "" + + +class TestVersionEntry: + """Frozen dataclass for plugin version entries.""" + + def test_basic_creation(self): + ve = VersionEntry(version="2.1.0", ref="abc123def456") + assert ve.version == "2.1.0" + assert ve.ref == "abc123def456" + + def test_frozen(self): + ve = VersionEntry(version="1.0.0", ref="deadbeef") + with pytest.raises(AttributeError): + ve.version = "2.0.0" + + def test_equality(self): + a = VersionEntry(version="1.0.0", ref="abc123") + b = VersionEntry(version="1.0.0", ref="abc123") + assert a == b + + def test_inequality(self): + a = VersionEntry(version="1.0.0", ref="abc123") + b = VersionEntry(version="1.0.0", ref="def456") + assert a != b + + +class TestMarketplacePluginVersions: + """MarketplacePlugin.versions field behavior.""" + + def test_default_versions_empty(self): + p = MarketplacePlugin(name="my-plugin") + assert p.versions == () + + def test_explicit_versions(self): + entries = ( + VersionEntry(version="2.1.0", ref="abc123"), + VersionEntry(version="2.0.0", ref="def456"), + ) + p = MarketplacePlugin(name="my-plugin", versions=entries) + assert len(p.versions) == 2 + assert p.versions[0].version == "2.1.0" + assert p.versions[1].ref == "def456" + + +class TestParseVersions: + """Parsing versions[] array in _parse_plugin_entry / parse_marketplace_json.""" + + def test_parse_plugin_with_versions(self): + data = { + "name": "Test", + "plugins": [ + { + "name": "skill-auth", + "source": { + "type": "github", + "repo": "acme/monorepo", + "path": "skills/auth", + }, + "versions": [ + {"version": "2.1.0", "ref": "abc123def456"}, + {"version": "2.0.0", "ref": "9f8e7d6c5b4a"}, + ], + } + ], + } + manifest = parse_marketplace_json(data, "acme") + p = manifest.plugins[0] + assert len(p.versions) == 2 + assert p.versions[0] == VersionEntry(version="2.1.0", ref="abc123def456") + assert p.versions[1] == VersionEntry(version="2.0.0", ref="9f8e7d6c5b4a") + + def test_parse_plugin_without_versions_backward_compat(self): + """Plugins without versions[] should default to empty tuple.""" + data = { + "name": "Test", + "plugins": [ + { + "name": "legacy-plugin", + "repository": "acme-org/legacy", + "version": "1.0.0", + } + ], + } + manifest = parse_marketplace_json(data, "test-mkt") + p = manifest.plugins[0] + assert p.versions == () + assert p.version == "1.0.0" + + def test_malformed_version_entries_skipped(self): + """Entries missing version or ref are silently skipped.""" + data = { + "name": "Test", + "plugins": [ + { + "name": "partial-versions", + "repository": "o/r", + "versions": [ + {"version": "2.1.0", "ref": "abc123"}, + {"version": "2.0.0"}, # Missing ref + {"ref": "deadbeef"}, # Missing version + "not-a-dict", # Non-dict entry + {"version": "", "ref": "abc"}, # Empty version + {"version": "1.0.0", "ref": ""}, # Empty ref + {"version": "1.5.0", "ref": "cafebabe"}, + ], + } + ], + } + manifest = parse_marketplace_json(data, "test") + p = manifest.plugins[0] + assert len(p.versions) == 2 + assert p.versions[0].version == "2.1.0" + assert p.versions[1].version == "1.5.0" + + def test_versions_not_a_list_ignored(self): + """Non-list versions field results in empty tuple.""" + data = { + "name": "Test", + "plugins": [ + { + "name": "bad-versions", + "repository": "o/r", + "versions": "not-a-list", + } + ], + } + manifest = parse_marketplace_json(data, "test") + p = manifest.plugins[0] + assert p.versions == () + + def test_versions_copilot_format(self): + """Versions work with Copilot CLI repository format too.""" + data = { + "name": "Test", + "plugins": [ + { + "name": "copilot-plugin", + "repository": "acme-org/plugin", + "ref": "v1.0.0", + "versions": [ + {"version": "1.0.0", "ref": "v1.0.0"}, + {"version": "0.9.0", "ref": "v0.9.0"}, + ], + } + ], + } + manifest = parse_marketplace_json(data, "copilot-mkt") + p = manifest.plugins[0] + assert len(p.versions) == 2 + assert p.versions[0].version == "1.0.0" + assert p.versions[0].ref == "v1.0.0" + + def test_roundtrip_parse_access_verify(self): + """Parse -> access versions -> verify values end-to-end.""" + data = { + "name": "Acme Marketplace", + "plugins": [ + { + "name": "skill-auth", + "description": "Auth skill", + "source": { + "type": "github", + "repo": "acme/monorepo", + "path": "skills/auth", + }, + "version": "2.1.0", + "tags": ["auth", "security"], + "versions": [ + {"version": "2.1.0", "ref": "abc123def456"}, + {"version": "2.0.0", "ref": "9f8e7d6c5b4a"}, + {"version": "1.0.0", "ref": "111111222222"}, + ], + }, + { + "name": "no-versions-plugin", + "repository": "acme/other", + }, + ], + } + manifest = parse_marketplace_json(data, "acme-mkt") + + # Plugin with versions + auth = manifest.find_plugin("skill-auth") + assert auth is not None + assert auth.version == "2.1.0" + assert auth.tags == ("auth", "security") + assert len(auth.versions) == 3 + assert auth.versions[0].version == "2.1.0" + assert auth.versions[0].ref == "abc123def456" + assert auth.versions[2].version == "1.0.0" + assert auth.versions[2].ref == "111111222222" + assert auth.source_marketplace == "acme-mkt" + + # Plugin without versions + other = manifest.find_plugin("no-versions-plugin") + assert other is not None + assert other.versions == () diff --git a/tests/unit/marketplace/test_marketplace_resolver.py b/tests/unit/marketplace/test_marketplace_resolver.py index 8bc20d3c..edf0edc6 100644 --- a/tests/unit/marketplace/test_marketplace_resolver.py +++ b/tests/unit/marketplace/test_marketplace_resolver.py @@ -21,28 +21,32 @@ def test_simple(self): assert parse_marketplace_ref("security-checks@acme-tools") == ( "security-checks", "acme-tools", + None, ) def test_dots(self): assert parse_marketplace_ref("my.plugin@my.marketplace") == ( "my.plugin", "my.marketplace", + None, ) def test_underscores(self): assert parse_marketplace_ref("my_plugin@my_marketplace") == ( "my_plugin", "my_marketplace", + None, ) def test_mixed(self): assert parse_marketplace_ref("plugin-v2.0@corp_tools") == ( "plugin-v2.0", "corp_tools", + None, ) def test_whitespace_stripped(self): - assert parse_marketplace_ref(" name@mkt ") == ("name", "mkt") + assert parse_marketplace_ref(" name@mkt ") == ("name", "mkt", None) # Negative cases -- not marketplace refs (should return None) def test_owner_repo(self): diff --git a/tests/unit/marketplace/test_versioned_resolver.py b/tests/unit/marketplace/test_versioned_resolver.py new file mode 100644 index 00000000..97e5c748 --- /dev/null +++ b/tests/unit/marketplace/test_versioned_resolver.py @@ -0,0 +1,387 @@ +"""Tests for version-aware marketplace resolution. + +Covers: +- parse_marketplace_ref with #version_spec suffix +- resolve_marketplace_plugin with versioned plugins +- resolve_marketplace_plugin backward compat (no versions) +- LockedDependency.version_spec round-trip serialization +""" + +from unittest.mock import MagicMock, patch + +import pytest + +from apm_cli.deps.lockfile import LockedDependency +from apm_cli.marketplace.models import ( + MarketplaceManifest, + MarketplacePlugin, + MarketplaceSource, + VersionEntry, +) +from apm_cli.marketplace.resolver import ( + parse_marketplace_ref, + resolve_marketplace_plugin, + resolve_plugin_source, +) + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + +def _make_versioned_plugin( + name="my-plugin", + repo="acme-org/my-plugin", + source_ref="main", + versions=None, +): + """Build a MarketplacePlugin with a github source and optional versions.""" + if versions is None: + versions = ( + VersionEntry(version="1.0.0", ref="v1.0.0"), + VersionEntry(version="1.1.0", ref="v1.1.0"), + VersionEntry(version="2.0.0", ref="v2.0.0"), + VersionEntry(version="2.1.0", ref="abc123def"), + ) + source = {"type": "github", "repo": repo, "ref": source_ref} + return MarketplacePlugin( + name=name, + source=source, + versions=versions, + source_marketplace="test-mkt", + ) + + +def _make_unversioned_plugin(name="legacy-plugin", repo="acme-org/legacy"): + """Build a MarketplacePlugin WITHOUT versions (backward compat).""" + return MarketplacePlugin( + name=name, + source={"type": "github", "repo": repo, "ref": "v1.0"}, + versions=(), + source_marketplace="test-mkt", + ) + + +def _make_manifest(*plugins, plugin_root=""): + return MarketplaceManifest( + name="test-mkt", + plugins=tuple(plugins), + plugin_root=plugin_root, + ) + + +def _make_source(): + return MarketplaceSource(name="test-mkt", owner="acme-org", repo="marketplace") + + +# --------------------------------------------------------------------------- +# parse_marketplace_ref -- version specifier suffix +# --------------------------------------------------------------------------- + + +class TestParseMarketplaceRefVersionSpec: + """Parsing NAME@MARKETPLACE#version_spec.""" + + def test_caret_specifier(self): + result = parse_marketplace_ref("plugin@mkt#^2.0.0") + assert result == ("plugin", "mkt", "^2.0.0") + + def test_tilde_specifier(self): + result = parse_marketplace_ref("plugin@mkt#~1.1.0") + assert result == ("plugin", "mkt", "~1.1.0") + + def test_exact_version(self): + result = parse_marketplace_ref("plugin@mkt#2.1.0") + assert result == ("plugin", "mkt", "2.1.0") + + def test_range_specifier(self): + result = parse_marketplace_ref("plugin@mkt#>=1.0.0,<3.0.0") + assert result == ("plugin", "mkt", ">=1.0.0,<3.0.0") + + def test_raw_git_ref(self): + result = parse_marketplace_ref("plugin@mkt#main") + assert result == ("plugin", "mkt", "main") + + def test_no_specifier(self): + result = parse_marketplace_ref("plugin@mkt") + assert result == ("plugin", "mkt", None) + + def test_empty_after_hash(self): + """Trailing # with nothing after is not a valid specifier.""" + result = parse_marketplace_ref("plugin@mkt#") + # The regex .+ requires at least 1 char after #, so # alone + # causes the full match to fail -> None. + assert result is None + + def test_whitespace_preserved_in_spec(self): + """Outer whitespace is stripped; inner spec is preserved.""" + result = parse_marketplace_ref(" plugin@mkt#^2.0.0 ") + assert result == ("plugin", "mkt", "^2.0.0") + + +# --------------------------------------------------------------------------- +# resolve_marketplace_plugin -- version-aware resolution +# --------------------------------------------------------------------------- + + +class TestResolveMarketplacePluginVersioned: + """Test resolve_marketplace_plugin when plugin has versions.""" + + def _resolve(self, plugin, version_spec=None): + """Call resolve_marketplace_plugin with mocked I/O.""" + manifest = _make_manifest(plugin) + source = _make_source() + + with ( + patch( + "apm_cli.marketplace.resolver.get_marketplace_by_name", + return_value=source, + ), + patch( + "apm_cli.marketplace.resolver.fetch_or_cache", + return_value=manifest, + ), + ): + return resolve_marketplace_plugin( + plugin.name, + "test-mkt", + version_spec=version_spec, + ) + + def test_caret_spec_resolves_highest_match(self): + """^2.0.0 should pick 2.1.0 (highest in ^2 range).""" + plugin = _make_versioned_plugin() + canonical, resolved = self._resolve(plugin, version_spec="^2.0.0") + assert canonical == "acme-org/my-plugin#abc123def" + assert resolved.name == "my-plugin" + + def test_exact_version_spec(self): + """Exact version 1.1.0 should pick exactly v1.1.0.""" + plugin = _make_versioned_plugin() + canonical, _ = self._resolve(plugin, version_spec="1.1.0") + assert canonical == "acme-org/my-plugin#v1.1.0" + + def test_tilde_spec(self): + """~1.0.0 should pick 1.1.0 (highest in ~1.0 range: >=1.0.0, <1.1.0). + Wait -- tilde means >=1.0.0, <1.1.0. So 1.0.0 is the only match.""" + plugin = _make_versioned_plugin() + canonical, _ = self._resolve(plugin, version_spec="~1.0.0") + assert canonical == "acme-org/my-plugin#v1.0.0" + + def test_no_spec_selects_latest(self): + """No version_spec (None) selects the highest available version.""" + plugin = _make_versioned_plugin() + canonical, _ = self._resolve(plugin, version_spec=None) + # 2.1.0 is the highest -> ref = abc123def + assert canonical == "acme-org/my-plugin#abc123def" + + def test_source_ref_overridden_by_version(self): + """The source.ref (main) should be replaced by the version entry ref.""" + plugin = _make_versioned_plugin(source_ref="main") + canonical, _ = self._resolve(plugin, version_spec="1.0.0") + # Source had #main, but version resolution should override to v1.0.0 + assert canonical == "acme-org/my-plugin#v1.0.0" + assert "#main" not in canonical + + def test_no_matching_version_raises(self): + """Specifier that matches nothing raises ValueError.""" + plugin = _make_versioned_plugin() + with pytest.raises(ValueError, match="No version matches"): + self._resolve(plugin, version_spec=">=99.0.0") + + def test_raw_git_ref_with_versions(self): + """A raw git ref (not semver) overrides the canonical ref directly.""" + plugin = _make_versioned_plugin(source_ref="main") + canonical, _ = self._resolve(plugin, version_spec="feature-branch") + assert canonical == "acme-org/my-plugin#feature-branch" + + +class TestResolveMarketplacePluginUnversioned: + """Test backward compat: plugins without versions use existing flow.""" + + def _resolve(self, plugin, version_spec=None): + manifest = _make_manifest(plugin) + source = _make_source() + + with ( + patch( + "apm_cli.marketplace.resolver.get_marketplace_by_name", + return_value=source, + ), + patch( + "apm_cli.marketplace.resolver.fetch_or_cache", + return_value=manifest, + ), + ): + return resolve_marketplace_plugin( + plugin.name, + "test-mkt", + version_spec=version_spec, + ) + + def test_unversioned_no_spec(self): + """Unversioned plugin without spec uses source.ref.""" + plugin = _make_unversioned_plugin() + canonical, resolved = self._resolve(plugin) + assert canonical == "acme-org/legacy#v1.0" + assert resolved.name == "legacy-plugin" + + def test_unversioned_with_spec_ignored(self): + """Unversioned plugin ignores version_spec -- uses source.ref.""" + plugin = _make_unversioned_plugin() + canonical, _ = self._resolve(plugin, version_spec="^2.0.0") + # No versions on plugin, so version_spec is silently ignored + assert canonical == "acme-org/legacy#v1.0" + + def test_unversioned_raw_ref_ignored(self): + """Unversioned plugin ignores raw ref -- uses source.ref.""" + plugin = _make_unversioned_plugin() + canonical, _ = self._resolve(plugin, version_spec="develop") + assert canonical == "acme-org/legacy#v1.0" + + +# --------------------------------------------------------------------------- +# Canonical string correctness +# --------------------------------------------------------------------------- + + +class TestCanonicalStringFromVersionEntry: + """Verify the canonical string is built correctly from version entry.""" + + def test_ref_replaces_source_ref(self): + """resolve_plugin_source produces owner/repo#source_ref; + version resolution should replace #source_ref with #entry.ref.""" + plugin = _make_versioned_plugin( + repo="org/repo", + source_ref="old-branch", + versions=(VersionEntry(version="3.0.0", ref="sha-abc"),), + ) + manifest = _make_manifest(plugin) + source = _make_source() + + with ( + patch( + "apm_cli.marketplace.resolver.get_marketplace_by_name", + return_value=source, + ), + patch( + "apm_cli.marketplace.resolver.fetch_or_cache", + return_value=manifest, + ), + ): + canonical, _ = resolve_marketplace_plugin( + "my-plugin", "test-mkt", version_spec="3.0.0" + ) + assert canonical == "org/repo#sha-abc" + + def test_source_without_ref_gets_version_ref(self): + """When source has no ref, canonical is owner/repo (no #). + Version resolution should append #entry.ref.""" + plugin = MarketplacePlugin( + name="no-ref-plugin", + source={"type": "github", "repo": "org/repo"}, + versions=(VersionEntry(version="1.0.0", ref="v1.0.0"),), + source_marketplace="test-mkt", + ) + manifest = _make_manifest(plugin) + source = _make_source() + + with ( + patch( + "apm_cli.marketplace.resolver.get_marketplace_by_name", + return_value=source, + ), + patch( + "apm_cli.marketplace.resolver.fetch_or_cache", + return_value=manifest, + ), + ): + canonical, _ = resolve_marketplace_plugin( + "no-ref-plugin", "test-mkt" + ) + assert canonical == "org/repo#v1.0.0" + + +# --------------------------------------------------------------------------- +# LockedDependency.version_spec serialization +# --------------------------------------------------------------------------- + + +class TestLockedDependencyVersionSpec: + """Verify version_spec field round-trips correctly in LockedDependency.""" + + def test_default_none(self): + dep = LockedDependency(repo_url="owner/repo") + assert dep.version_spec is None + + def test_to_dict_omits_none(self): + dep = LockedDependency(repo_url="owner/repo") + d = dep.to_dict() + assert "version_spec" not in d + + def test_to_dict_includes_value(self): + dep = LockedDependency(repo_url="owner/repo", version_spec="^2.0.0") + d = dep.to_dict() + assert d["version_spec"] == "^2.0.0" + + def test_from_dict_missing_field(self): + """Old lockfiles without version_spec still deserialize.""" + dep = LockedDependency.from_dict({"repo_url": "owner/repo"}) + assert dep.version_spec is None + + def test_from_dict_with_field(self): + dep = LockedDependency.from_dict({ + "repo_url": "owner/repo", + "version_spec": "~1.5.0", + }) + assert dep.version_spec == "~1.5.0" + + def test_roundtrip(self): + original = LockedDependency( + repo_url="owner/repo", + resolved_commit="abc123", + resolved_ref="v2.1.0", + discovered_via="acme-tools", + marketplace_plugin_name="my-plugin", + version_spec="^2.0.0", + ) + restored = LockedDependency.from_dict(original.to_dict()) + assert restored.version_spec == "^2.0.0" + assert restored.discovered_via == "acme-tools" + assert restored.marketplace_plugin_name == "my-plugin" + assert restored.resolved_commit == "abc123" + assert restored.resolved_ref == "v2.1.0" + + def test_backward_compat_existing_fields(self): + """Existing fields still work alongside version_spec.""" + dep = LockedDependency.from_dict({ + "repo_url": "owner/repo", + "resolved_commit": "abc123", + "content_hash": "sha256:def456", + "is_dev": True, + "discovered_via": "mkt", + "version_spec": ">=1.0.0,<3.0.0", + }) + assert dep.resolved_commit == "abc123" + assert dep.content_hash == "sha256:def456" + assert dep.is_dev is True + assert dep.discovered_via == "mkt" + assert dep.version_spec == ">=1.0.0,<3.0.0" + + def test_yaml_lockfile_roundtrip(self): + """version_spec survives a full YAML lockfile write/read cycle.""" + from apm_cli.deps.lockfile import LockFile + + lock = LockFile() + lock.add_dependency(LockedDependency( + repo_url="owner/repo", + version_spec="^2.0.0", + discovered_via="acme-tools", + )) + + yaml_str = lock.to_yaml() + restored = LockFile.from_yaml(yaml_str) + dep = restored.get_dependency("owner/repo") + assert dep is not None + assert dep.version_spec == "^2.0.0" + assert dep.discovered_via == "acme-tools" diff --git a/tests/unit/test_outdated_marketplace.py b/tests/unit/test_outdated_marketplace.py new file mode 100644 index 00000000..0b66b7a0 --- /dev/null +++ b/tests/unit/test_outdated_marketplace.py @@ -0,0 +1,464 @@ +"""Unit tests for marketplace-based version checking in ``apm outdated``.""" + +from unittest.mock import MagicMock, patch + +import pytest + +from apm_cli.commands.outdated import ( + _check_marketplace_versions, + _check_one_dep, +) +from apm_cli.deps.lockfile import LockedDependency +from apm_cli.marketplace.models import ( + MarketplaceManifest, + MarketplacePlugin, + MarketplaceSource, + VersionEntry, +) +from apm_cli.models.dependency.types import GitReferenceType, RemoteRef + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _marketplace_dep( + repo_url="acme-org/skill-auth", + discovered_via="acme-tools", + marketplace_plugin_name="skill-auth", + version="2.1.0", + resolved_ref="v2.1.0", + resolved_commit="aaa111", + host=None, +): + """Build a marketplace-sourced LockedDependency.""" + return LockedDependency( + repo_url=repo_url, + host=host, + resolved_ref=resolved_ref, + resolved_commit=resolved_commit, + discovered_via=discovered_via, + marketplace_plugin_name=marketplace_plugin_name, + version=version, + ) + + +def _git_dep( + repo_url="org/some-repo", + resolved_ref="v1.0.0", + resolved_commit="abc1234", + host=None, +): + """Build a plain git-sourced LockedDependency (no marketplace).""" + return LockedDependency( + repo_url=repo_url, + host=host, + resolved_ref=resolved_ref, + resolved_commit=resolved_commit, + ) + + +def _make_source(name="acme-tools"): + """Build a MarketplaceSource.""" + return MarketplaceSource( + name=name, owner="acme-org", repo="marketplace", + ) + + +def _make_manifest(name="acme-tools", plugins=None): + """Build a MarketplaceManifest.""" + return MarketplaceManifest( + name=name, + plugins=tuple(plugins or []), + ) + + +def _make_plugin(name="skill-auth", versions=None): + """Build a MarketplacePlugin with version entries.""" + entries = tuple( + VersionEntry(version=v, ref=f"v{v}") + for v in (versions or []) + ) + return MarketplacePlugin( + name=name, + source={"type": "github", "repo": "acme-org/skill-auth"}, + versions=entries, + source_marketplace="acme-tools", + ) + + +def _remote_tag(name, sha="abc123"): + """Build a RemoteRef tag.""" + return RemoteRef(name=name, ref_type=GitReferenceType.TAG, commit_sha=sha) + + +# Patch targets -- marketplace imports are lazy (inside function body) +_PATCH_GET_MKT = "apm_cli.marketplace.registry.get_marketplace_by_name" +_PATCH_FETCH = "apm_cli.marketplace.client.fetch_or_cache" + + +# --------------------------------------------------------------------------- +# Tests: _check_marketplace_versions +# --------------------------------------------------------------------------- + +class TestCheckMarketplaceVersions: + """Tests for the ``_check_marketplace_versions`` helper.""" + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_newer_version_available(self, mock_get_mkt, mock_fetch): + """Marketplace dep with a newer version reports as outdated.""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("skill-auth", ["2.1.0", "3.0.0"])], + ) + dep = _marketplace_dep(version="2.1.0") + + result = _check_marketplace_versions(dep, verbose=False) + + assert result is not None + package, current, latest, status, extra, source = result + assert package == "skill-auth@acme-tools" + assert current == "2.1.0" + assert latest == "3.0.0" + assert status == "outdated" + assert source == "marketplace: acme-tools" + assert extra == [] + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_already_at_latest(self, mock_get_mkt, mock_fetch): + """Marketplace dep at latest version reports as up-to-date.""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("skill-auth", ["2.1.0", "3.0.0"])], + ) + dep = _marketplace_dep(version="3.0.0") + + result = _check_marketplace_versions(dep, verbose=False) + + assert result is not None + package, current, latest, status, extra, source = result + assert package == "skill-auth@acme-tools" + assert current == "3.0.0" + assert latest == "3.0.0" + assert status == "up-to-date" + assert source == "marketplace: acme-tools" + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_no_versions_falls_through(self, mock_get_mkt, mock_fetch): + """Plugin with empty versions[] returns None (fall through to git).""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("skill-auth", versions=[])], + ) + dep = _marketplace_dep(version="2.1.0") + + result = _check_marketplace_versions(dep, verbose=False) + assert result is None + + def test_non_marketplace_dep_returns_none(self): + """Dep without discovered_via returns None immediately.""" + dep = _git_dep() + result = _check_marketplace_versions(dep, verbose=False) + assert result is None + + @patch(_PATCH_GET_MKT) + def test_marketplace_not_found_falls_through(self, mock_get_mkt): + """MarketplaceNotFoundError returns None with a warning.""" + from apm_cli.marketplace.errors import MarketplaceNotFoundError + + mock_get_mkt.side_effect = MarketplaceNotFoundError("acme-tools") + dep = _marketplace_dep(version="2.1.0") + + result = _check_marketplace_versions(dep, verbose=False) + assert result is None + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_fetch_error_falls_through(self, mock_get_mkt, mock_fetch): + """MarketplaceFetchError returns None with a warning.""" + from apm_cli.marketplace.errors import MarketplaceFetchError + + mock_get_mkt.return_value = _make_source() + mock_fetch.side_effect = MarketplaceFetchError("acme-tools", "timeout") + dep = _marketplace_dep(version="2.1.0") + + result = _check_marketplace_versions(dep, verbose=False) + assert result is None + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_plugin_not_found_falls_through(self, mock_get_mkt, mock_fetch): + """Plugin name not in manifest returns None (fall through).""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("other-plugin", ["1.0.0"])], + ) + dep = _marketplace_dep( + marketplace_plugin_name="skill-auth", version="2.1.0", + ) + + result = _check_marketplace_versions(dep, verbose=False) + assert result is None + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_verbose_shows_version_list(self, mock_get_mkt, mock_fetch): + """In verbose mode, extra contains available version strings.""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("skill-auth", ["1.0.0", "2.0.0", "3.0.0"])], + ) + dep = _marketplace_dep(version="1.0.0") + + result = _check_marketplace_versions(dep, verbose=True) + + assert result is not None + _, _, _, status, extra, _ = result + assert status == "outdated" + assert "1.0.0" in extra + assert "3.0.0" in extra + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_dep_without_version_falls_through(self, mock_get_mkt, mock_fetch): + """Marketplace dep with empty version string returns None.""" + dep = _marketplace_dep(version="") + + result = _check_marketplace_versions(dep, verbose=False) + assert result is None + # Should NOT call marketplace APIs when version is empty + mock_get_mkt.assert_not_called() + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_invalid_current_version_falls_through( + self, mock_get_mkt, mock_fetch, + ): + """Non-semver current version returns None (fall through).""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("skill-auth", ["2.1.0"])], + ) + dep = _marketplace_dep(version="not-a-version") + + result = _check_marketplace_versions(dep, verbose=False) + assert result is None + + +# --------------------------------------------------------------------------- +# Tests: _check_one_dep integration with marketplace +# --------------------------------------------------------------------------- + +class TestCheckOneDepMarketplace: + """Tests for ``_check_one_dep`` marketplace integration.""" + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_marketplace_dep_skips_git(self, mock_get_mkt, mock_fetch): + """Marketplace dep with versions does NOT call the git downloader.""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("skill-auth", ["2.1.0", "3.0.0"])], + ) + dep = _marketplace_dep(version="2.1.0") + downloader = MagicMock() + + result = _check_one_dep(dep, downloader, verbose=False) + + package, current, latest, status, extra, source = result + assert status == "outdated" + assert source == "marketplace: acme-tools" + # Git downloader should NOT have been called + downloader.list_remote_refs.assert_not_called() + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_marketplace_fallback_to_git(self, mock_get_mkt, mock_fetch): + """Marketplace dep with no versions falls back to git check.""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("skill-auth", versions=[])], + ) + dep = _marketplace_dep(version="2.1.0") + downloader = MagicMock() + downloader.list_remote_refs.return_value = [ + _remote_tag("v2.1.0", sha="aaa111"), + _remote_tag("v3.0.0", sha="bbb222"), + ] + + result = _check_one_dep(dep, downloader, verbose=False) + + _, _, _, status, _, source = result + # Should have fallen back to git-based check + assert source == "git tags" + downloader.list_remote_refs.assert_called_once() + + def test_git_dep_uses_git_check(self): + """Non-marketplace dep goes through git path and includes source.""" + dep = _git_dep(resolved_ref="v1.0.0", resolved_commit="aaa111") + downloader = MagicMock() + downloader.list_remote_refs.return_value = [ + _remote_tag("v2.0.0", sha="bbb222"), + _remote_tag("v1.0.0", sha="aaa111"), + ] + + result = _check_one_dep(dep, downloader, verbose=False) + + package, current, latest, status, extra, source = result + assert source == "git tags" + assert status == "outdated" + downloader.list_remote_refs.assert_called_once() + + +# --------------------------------------------------------------------------- +# Tests: mixed marketplace + git deps +# --------------------------------------------------------------------------- + +class TestMixedDeps: + """Both marketplace and git deps are checked correctly.""" + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_mixed_deps_checked_correctly(self, mock_get_mkt, mock_fetch): + """Marketplace deps use marketplace, git deps use git.""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("skill-auth", ["2.1.0", "3.0.0"])], + ) + + mkt_dep = _marketplace_dep(version="2.1.0") + git_dep = _git_dep(resolved_ref="v1.0.0", resolved_commit="aaa111") + + downloader = MagicMock() + downloader.list_remote_refs.return_value = [ + _remote_tag("v1.0.0", sha="aaa111"), + ] + + mkt_result = _check_one_dep(mkt_dep, downloader, verbose=False) + git_result = _check_one_dep(git_dep, downloader, verbose=False) + + # Marketplace dep + _, _, _, mkt_status, _, mkt_source = mkt_result + assert mkt_source == "marketplace: acme-tools" + assert mkt_status == "outdated" + + # Git dep + _, _, _, git_status, _, git_source = git_result + assert git_source == "git tags" + assert git_status == "up-to-date" + + # Git downloader called only for the git dep + assert downloader.list_remote_refs.call_count == 1 + + +# --------------------------------------------------------------------------- +# Tests: version_spec handling +# --------------------------------------------------------------------------- + +class TestVersionSpec: + """Tests for version_spec range checking.""" + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_latest_outside_range(self, mock_get_mkt, mock_fetch): + """Latest version outside version_spec is reported with annotation.""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("skill-auth", ["2.1.0", "3.0.0"])], + ) + dep = _marketplace_dep(version="2.1.0") + # Simulate version_spec field from parallel task + dep.version_spec = "^2.0.0" # type: ignore[attr-defined] + + result = _check_marketplace_versions(dep, verbose=False) + + assert result is not None + _, _, latest, status, _, _ = result + assert status == "outdated" + # 3.0.0 is outside ^2.0.0 range + assert "outside range" in latest + assert "^2.0.0" in latest + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_latest_within_range(self, mock_get_mkt, mock_fetch): + """Latest version within version_spec shows plain version.""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("skill-auth", ["2.1.0", "2.5.0"])], + ) + dep = _marketplace_dep(version="2.1.0") + dep.version_spec = "^2.0.0" # type: ignore[attr-defined] + + result = _check_marketplace_versions(dep, verbose=False) + + assert result is not None + _, _, latest, status, _, _ = result + assert status == "outdated" + assert latest == "2.5.0" + assert "outside range" not in latest + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_no_version_spec_plain_display(self, mock_get_mkt, mock_fetch): + """Without version_spec, latest version shown without annotation.""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("skill-auth", ["2.1.0", "3.0.0"])], + ) + dep = _marketplace_dep(version="2.1.0") + + result = _check_marketplace_versions(dep, verbose=False) + + assert result is not None + _, _, latest, status, _, _ = result + assert status == "outdated" + assert latest == "3.0.0" + assert "outside range" not in latest + + +# --------------------------------------------------------------------------- +# Tests: result tuple shape +# --------------------------------------------------------------------------- + +class TestResultTupleShape: + """All code paths produce 6-element tuples.""" + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_marketplace_result_has_six_elements( + self, mock_get_mkt, mock_fetch, + ): + """Marketplace result tuple has (pkg, current, latest, status, extra, source).""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("skill-auth", ["2.1.0", "3.0.0"])], + ) + dep = _marketplace_dep(version="2.1.0") + + result = _check_marketplace_versions(dep, verbose=False) + assert result is not None + assert len(result) == 6 + + def test_git_tag_result_has_six_elements(self): + """Git tag check result tuple has 6 elements.""" + dep = _git_dep(resolved_ref="v1.0.0", resolved_commit="aaa111") + downloader = MagicMock() + downloader.list_remote_refs.return_value = [ + _remote_tag("v1.0.0", sha="aaa111"), + ] + + result = _check_one_dep(dep, downloader, verbose=False) + assert len(result) == 6 + + def test_git_unknown_result_has_six_elements(self): + """Unknown git result tuple has 6 elements.""" + dep = _git_dep(resolved_ref="v1.0.0") + downloader = MagicMock() + downloader.list_remote_refs.side_effect = Exception("network error") + + result = _check_one_dep(dep, downloader, verbose=False) + assert len(result) == 6 diff --git a/tests/unit/test_version_resolver.py b/tests/unit/test_version_resolver.py new file mode 100644 index 00000000..636436ff --- /dev/null +++ b/tests/unit/test_version_resolver.py @@ -0,0 +1,322 @@ +"""Tests for marketplace version resolver -- semver range resolution.""" + +import pytest + +from apm_cli.marketplace.models import VersionEntry +from apm_cli.marketplace.version_resolver import ( + is_version_specifier, + resolve_version, +) + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + +def _v(version: str, ref: str = "") -> VersionEntry: + """Shorthand to build a VersionEntry with a default ref.""" + return VersionEntry(version=version, ref=ref or f"sha-{version}") + + +# A realistic set of versions in *shuffled* order so tests verify that the +# resolver sorts internally rather than relying on input order. +SAMPLE_VERSIONS = [ + _v("2.1.0", "abc111"), + _v("1.0.0", "abc000"), + _v("3.0.0", "abc300"), + _v("0.5.0", "abc050"), + _v("2.0.0", "abc200"), + _v("1.5.0", "abc150"), + _v("0.5.3", "abc053"), + _v("2.1.1", "abc211"), + _v("0.0.3", "abc003"), +] + + +# --------------------------------------------------------------------------- +# resolve_version -- latest (None / empty) +# --------------------------------------------------------------------------- + + +class TestResolveLatest: + """When specifier is None or empty, return the highest semver.""" + + def test_none_returns_highest(self): + result = resolve_version(None, SAMPLE_VERSIONS) + assert result.version == "3.0.0" + + def test_empty_string_returns_highest(self): + result = resolve_version("", SAMPLE_VERSIONS) + assert result.version == "3.0.0" + + def test_whitespace_only_returns_highest(self): + result = resolve_version(" ", SAMPLE_VERSIONS) + assert result.version == "3.0.0" + + +# --------------------------------------------------------------------------- +# resolve_version -- exact match +# --------------------------------------------------------------------------- + + +class TestResolveExact: + """Exact version specifiers like ``"2.1.0"``.""" + + def test_exact_match(self): + result = resolve_version("2.1.0", SAMPLE_VERSIONS) + assert result.version == "2.1.0" + assert result.ref == "abc111" + + def test_exact_match_lowest(self): + result = resolve_version("1.0.0", SAMPLE_VERSIONS) + assert result.version == "1.0.0" + + def test_exact_no_match_raises(self): + with pytest.raises(ValueError, match="No version matches"): + resolve_version("99.0.0", SAMPLE_VERSIONS) + + +# --------------------------------------------------------------------------- +# resolve_version -- caret (^) +# --------------------------------------------------------------------------- + + +class TestResolveCaret: + """Caret specifier ``^X.Y.Z`` -- compatible-with semantics.""" + + def test_caret_major_nonzero(self): + # ^2.0.0 -> >=2.0.0, <3.0.0 => should pick 2.1.1 + result = resolve_version("^2.0.0", SAMPLE_VERSIONS) + assert result.version == "2.1.1" + + def test_caret_includes_exact(self): + # ^2.1.1 -> >=2.1.1, <3.0.0 => only 2.1.1 qualifies + result = resolve_version("^2.1.1", SAMPLE_VERSIONS) + assert result.version == "2.1.1" + + def test_caret_zero_major(self): + # ^0.5.0 -> >=0.5.0, <0.6.0 => picks 0.5.3 + result = resolve_version("^0.5.0", SAMPLE_VERSIONS) + assert result.version == "0.5.3" + + def test_caret_zero_major_zero_minor(self): + # ^0.0.3 -> >=0.0.3, <0.0.4 => picks 0.0.3 + result = resolve_version("^0.0.3", SAMPLE_VERSIONS) + assert result.version == "0.0.3" + + def test_caret_no_match(self): + # ^5.0.0 -> >=5.0.0, <6.0.0 => nothing + with pytest.raises(ValueError, match="No version matches"): + resolve_version("^5.0.0", SAMPLE_VERSIONS) + + +# --------------------------------------------------------------------------- +# resolve_version -- tilde (~) +# --------------------------------------------------------------------------- + + +class TestResolveTilde: + """Tilde specifier ``~X.Y.Z`` -- patch-level changes only.""" + + def test_tilde_basic(self): + # ~2.1.0 -> >=2.1.0, <2.2.0 => picks 2.1.1 + result = resolve_version("~2.1.0", SAMPLE_VERSIONS) + assert result.version == "2.1.1" + + def test_tilde_exact_patch(self): + # ~2.1.1 -> >=2.1.1, <2.2.0 => picks 2.1.1 + result = resolve_version("~2.1.1", SAMPLE_VERSIONS) + assert result.version == "2.1.1" + + def test_tilde_no_match(self): + # ~2.2.0 -> >=2.2.0, <2.3.0 => nothing in sample + with pytest.raises(ValueError, match="No version matches"): + resolve_version("~2.2.0", SAMPLE_VERSIONS) + + +# --------------------------------------------------------------------------- +# resolve_version -- comparison operators +# --------------------------------------------------------------------------- + + +class TestResolveComparison: + """Comparison specifiers like ``>=``, ``>``, ``<``, ``<=``.""" + + def test_gte(self): + # >=1.5.0 -> picks 3.0.0 (highest above 1.5.0) + result = resolve_version(">=1.5.0", SAMPLE_VERSIONS) + assert result.version == "3.0.0" + + def test_gt(self): + # >2.1.0 -> picks 3.0.0 (strictly greater) + result = resolve_version(">2.1.0", SAMPLE_VERSIONS) + assert result.version == "3.0.0" + + def test_lte(self): + # <=1.0.0 -> picks 1.0.0 + result = resolve_version("<=1.0.0", SAMPLE_VERSIONS) + assert result.version == "1.0.0" + + def test_lt(self): + # <1.0.0 -> picks 0.5.3 + result = resolve_version("<1.0.0", SAMPLE_VERSIONS) + assert result.version == "0.5.3" + + +# --------------------------------------------------------------------------- +# resolve_version -- compound ranges +# --------------------------------------------------------------------------- + + +class TestResolveCompound: + """Compound specifiers with comma-separated clauses.""" + + def test_range_inclusive(self): + # >=1.0.0,<3.0.0 -> highest in [1.0.0, 3.0.0) => 2.1.1 + result = resolve_version(">=1.0.0,<3.0.0", SAMPLE_VERSIONS) + assert result.version == "2.1.1" + + def test_range_with_spaces(self): + # Whitespace around commas and operators + result = resolve_version(" >=1.0.0 , <3.0.0 ", SAMPLE_VERSIONS) + assert result.version == "2.1.1" + + def test_range_tight(self): + # >=2.0.0,<=2.1.0 -> picks 2.1.0 + result = resolve_version(">=2.0.0,<=2.1.0", SAMPLE_VERSIONS) + assert result.version == "2.1.0" + + +# --------------------------------------------------------------------------- +# resolve_version -- edge cases +# --------------------------------------------------------------------------- + + +class TestResolveEdgeCases: + """Edge cases and error handling.""" + + def test_empty_list_raises(self): + with pytest.raises(ValueError, match="No versions available"): + resolve_version(None, []) + + def test_single_version_none_specifier(self): + result = resolve_version(None, [_v("1.0.0")]) + assert result.version == "1.0.0" + + def test_single_version_exact_match(self): + result = resolve_version("1.0.0", [_v("1.0.0")]) + assert result.version == "1.0.0" + + def test_single_version_no_match(self): + with pytest.raises(ValueError, match="No version matches"): + resolve_version("2.0.0", [_v("1.0.0")]) + + def test_invalid_semver_entries_skipped(self): + """Entries with non-semver version strings are silently skipped.""" + versions = [ + _v("1.0.0"), + VersionEntry(version="not-a-version", ref="bad"), + VersionEntry(version="main", ref="bad2"), + _v("2.0.0"), + ] + result = resolve_version(None, versions) + assert result.version == "2.0.0" + + def test_all_invalid_semver_raises(self): + """If every entry has an invalid version string, raise ValueError.""" + versions = [ + VersionEntry(version="main", ref="aaa"), + VersionEntry(version="bad", ref="bbb"), + ] + with pytest.raises(ValueError, match="No version matches"): + resolve_version(None, versions) + + def test_unordered_input(self): + """Versions not in order should still resolve correctly.""" + versions = [_v("3.0.0"), _v("1.0.0"), _v("2.0.0")] + result = resolve_version("^2.0.0", versions) + assert result.version == "2.0.0" + + def test_preserves_entry_identity(self): + """The returned VersionEntry is the exact object from the input.""" + entry = _v("2.1.0", "specific-ref") + versions = [_v("1.0.0"), entry, _v("3.0.0")] + result = resolve_version("2.1.0", versions) + assert result is entry + + def test_error_message_includes_available(self): + """ValueError message should list available versions.""" + versions = [_v("1.0.0"), _v("2.0.0")] + with pytest.raises(ValueError, match="1.0.0"): + resolve_version("99.0.0", versions) + + +# --------------------------------------------------------------------------- +# is_version_specifier +# --------------------------------------------------------------------------- + + +class TestIsVersionSpecifier: + """Heuristic to distinguish version specifiers from git refs.""" + + # Positive cases -- should be recognized as version specifiers + def test_exact_version(self): + assert is_version_specifier("2.1.0") is True + + def test_caret(self): + assert is_version_specifier("^2.0.0") is True + + def test_tilde(self): + assert is_version_specifier("~2.1.0") is True + + def test_gte(self): + assert is_version_specifier(">=1.5.0") is True + + def test_gt(self): + assert is_version_specifier(">1.0.0") is True + + def test_lte(self): + assert is_version_specifier("<=3.0.0") is True + + def test_lt(self): + assert is_version_specifier("<3.0.0") is True + + def test_eq(self): + assert is_version_specifier("==2.0.0") is True + + def test_compound(self): + assert is_version_specifier(">=1.0.0,<3.0.0") is True + + def test_compound_with_spaces(self): + assert is_version_specifier(" >=1.0.0 , <3.0.0 ") is True + + # Negative cases -- should NOT be recognized as version specifiers + def test_branch_name(self): + assert is_version_specifier("main") is False + + def test_sha_like(self): + assert is_version_specifier("abc123def") is False + + def test_feature_branch(self): + assert is_version_specifier("feature/my-branch") is False + + def test_tag_with_v_prefix(self): + # "v2.1.0" is a git tag, not a bare specifier + assert is_version_specifier("v2.1.0") is False + + def test_empty_string(self): + assert is_version_specifier("") is False + + def test_whitespace_only(self): + assert is_version_specifier(" ") is False + + def test_partial_version(self): + # "2.1" is not X.Y.Z + assert is_version_specifier("2.1") is False + + def test_hex_string(self): + # 40-char hex could be a SHA + assert is_version_specifier("a" * 40) is False + + def test_alpha_prefix_with_digits(self): + assert is_version_specifier("release-2.0") is False diff --git a/tests/unit/test_view_versions.py b/tests/unit/test_view_versions.py new file mode 100644 index 00000000..40102d24 --- /dev/null +++ b/tests/unit/test_view_versions.py @@ -0,0 +1,396 @@ +"""Tests for marketplace-based version display in ``apm view NAME@MARKETPLACE versions``. + +Covers the ``_display_marketplace_versions()`` path added to +``src/apm_cli/commands/view.py``. All marketplace interactions are +mocked -- no network calls. +""" + +import contextlib +import sys +import types +from unittest.mock import MagicMock, patch + +import pytest +from click.testing import CliRunner + +from apm_cli.cli import cli +from apm_cli.marketplace.errors import ( + MarketplaceFetchError, + MarketplaceNotFoundError, + PluginNotFoundError, +) +from apm_cli.marketplace.models import ( + MarketplaceManifest, + MarketplacePlugin, + MarketplaceSource, + VersionEntry, +) + + +# ------------------------------------------------------------------ +# Rich-fallback helper (same approach as test_view_command.py) +# ------------------------------------------------------------------ + + +def _force_rich_fallback(): + """Context-manager that forces the text-only code path.""" + + @contextlib.contextmanager + def _ctx(): + keys = [ + "rich", + "rich.console", + "rich.table", + "rich.tree", + "rich.panel", + "rich.text", + ] + originals = {k: sys.modules.get(k) for k in keys} + + for k in keys: + stub = types.ModuleType(k) + stub.__path__ = [] + + def _raise(name, _k=k): + raise ImportError(f"rich not available in test: {_k}") + + stub.__getattr__ = _raise + sys.modules[k] = stub + + try: + yield + finally: + for k, v in originals.items(): + if v is None: + sys.modules.pop(k, None) + else: + sys.modules[k] = v + + return _ctx() + + +# ------------------------------------------------------------------ +# Fixtures +# ------------------------------------------------------------------ + +_DUMMY_SOURCE = MarketplaceSource( + name="acme-tools", + owner="acme", + repo="marketplace", +) + + +def _make_manifest(plugins): + """Helper to build a MarketplaceManifest with the given plugins list.""" + return MarketplaceManifest( + name="acme-tools", + plugins=tuple(plugins), + ) + + +def _make_plugin(name, versions=(), **kwargs): + """Helper to build a MarketplacePlugin with version entries.""" + version_entries = tuple( + VersionEntry(version=v, ref=r) for v, r in versions + ) + return MarketplacePlugin( + name=name, + source={"type": "github", "repo": "acme/plugin"}, + versions=version_entries, + **kwargs, + ) + + +# Common patches for marketplace path -- target the source modules since +# _display_marketplace_versions() uses local imports. +_PATCH_GET_MARKETPLACE = "apm_cli.marketplace.registry.get_marketplace_by_name" +_PATCH_FETCH_OR_CACHE = "apm_cli.marketplace.client.fetch_or_cache" + + +# ------------------------------------------------------------------ +# Tests +# ------------------------------------------------------------------ + + +class TestMarketplaceVersionDisplay: + """Tests for marketplace-based version display via CLI.""" + + def setup_method(self): + self.runner = CliRunner() + + # -- Happy path: plugin with versions --------------------------------- + + def test_marketplace_versions_rich_table(self): + """``apm view plugin@marketplace versions`` renders a version table.""" + plugin = _make_plugin( + "my-plugin", + versions=[ + ("1.0.0", "abc1234"), + ("2.0.0", "def5678"), + ("1.5.0", "bbb9999"), + ], + ) + manifest = _make_manifest([plugin]) + + with patch( + "apm_cli.marketplace.registry.get_marketplace_by_name", + return_value=_DUMMY_SOURCE, + ), patch( + "apm_cli.marketplace.client.fetch_or_cache", + return_value=manifest, + ): + with _force_rich_fallback(): + result = self.runner.invoke( + cli, ["view", "my-plugin@acme-tools", "versions"] + ) + + assert result.exit_code == 0 + output = result.output + # Title + assert "my-plugin" in output + assert "acme-tools" in output + # All versions present + assert "2.0.0" in output + assert "1.5.0" in output + assert "1.0.0" in output + # Refs present + assert "def5678" in output + assert "bbb9999" in output + assert "abc1234" in output + # Latest badge + assert "latest" in output + # Install hints + assert "apm install my-plugin@acme-tools" in output + + # -- Versions sorted descending by semver ----------------------------- + + def test_marketplace_versions_sorted_descending(self): + """Versions are sorted by semver descending (newest first).""" + plugin = _make_plugin( + "sorted-plugin", + versions=[ + ("1.0.0", "ref-100"), + ("3.0.0", "ref-300"), + ("2.0.0", "ref-200"), + ("2.5.0", "ref-250"), + ], + ) + manifest = _make_manifest([plugin]) + + with patch( + "apm_cli.marketplace.registry.get_marketplace_by_name", + return_value=_DUMMY_SOURCE, + ), patch( + "apm_cli.marketplace.client.fetch_or_cache", + return_value=manifest, + ): + with _force_rich_fallback(): + result = self.runner.invoke( + cli, ["view", "sorted-plugin@acme-tools", "versions"] + ) + + assert result.exit_code == 0 + output = result.output + # Find positions -- 3.0.0 should come before 2.5.0, etc. + pos_300 = output.index("3.0.0") + pos_250 = output.index("2.5.0") + pos_200 = output.index("2.0.0") + pos_100 = output.index("1.0.0") + assert pos_300 < pos_250 < pos_200 < pos_100 + + # -- Latest badge only on highest semver ------------------------------ + + def test_latest_badge_on_highest_semver(self): + """Only the highest semver version gets the 'latest' badge.""" + plugin = _make_plugin( + "badge-plugin", + versions=[ + ("1.0.0", "ref-old"), + ("2.0.0", "ref-new"), + ], + ) + manifest = _make_manifest([plugin]) + + with patch( + "apm_cli.marketplace.registry.get_marketplace_by_name", + return_value=_DUMMY_SOURCE, + ), patch( + "apm_cli.marketplace.client.fetch_or_cache", + return_value=manifest, + ): + with _force_rich_fallback(): + result = self.runner.invoke( + cli, ["view", "badge-plugin@acme-tools", "versions"] + ) + + assert result.exit_code == 0 + # "latest" should appear exactly once + assert result.output.count("latest") == 1 + + # -- Empty versions --------------------------------------------------- + + def test_marketplace_plugin_empty_versions(self): + """Plugin with empty versions[] shows informational message.""" + plugin = _make_plugin("empty-plugin", versions=[]) + manifest = _make_manifest([plugin]) + + with patch( + "apm_cli.marketplace.registry.get_marketplace_by_name", + return_value=_DUMMY_SOURCE, + ), patch( + "apm_cli.marketplace.client.fetch_or_cache", + return_value=manifest, + ): + result = self.runner.invoke( + cli, ["view", "empty-plugin@acme-tools", "versions"] + ) + + assert result.exit_code == 0 + assert "no version history" in result.output.lower() + assert "single-ref" in result.output.lower() + + # -- Plugin not found in marketplace ---------------------------------- + + def test_marketplace_plugin_not_found(self): + """Plugin not in marketplace exits 1 with error message.""" + manifest = _make_manifest([]) # No plugins + + with patch( + "apm_cli.marketplace.registry.get_marketplace_by_name", + return_value=_DUMMY_SOURCE, + ), patch( + "apm_cli.marketplace.client.fetch_or_cache", + return_value=manifest, + ): + result = self.runner.invoke( + cli, ["view", "missing-plugin@acme-tools", "versions"] + ) + + assert result.exit_code == 1 + assert "missing-plugin" in result.output.lower() + assert "not found" in result.output.lower() + + # -- Marketplace not registered --------------------------------------- + + def test_marketplace_not_registered(self): + """Unregistered marketplace exits 1 with helpful error.""" + with patch( + "apm_cli.marketplace.registry.get_marketplace_by_name", + side_effect=MarketplaceNotFoundError("unknown-mkt"), + ): + result = self.runner.invoke( + cli, ["view", "plugin@unknown-mkt", "versions"] + ) + + assert result.exit_code == 1 + assert "unknown-mkt" in result.output.lower() + assert "not registered" in result.output.lower() or "marketplace" in result.output.lower() + + # -- Network error fetching marketplace ------------------------------- + + def test_marketplace_fetch_error(self): + """Network error exits 1 with suggestion to check network.""" + with patch( + "apm_cli.marketplace.registry.get_marketplace_by_name", + return_value=_DUMMY_SOURCE, + ), patch( + "apm_cli.marketplace.client.fetch_or_cache", + side_effect=MarketplaceFetchError("acme-tools", "connection timeout"), + ): + result = self.runner.invoke( + cli, ["view", "plugin@acme-tools", "versions"] + ) + + assert result.exit_code == 1 + assert "failed to fetch" in result.output.lower() + assert "check your network" in result.output.lower() or "try again" in result.output.lower() + + # -- Non-marketplace package falls through to git flow ---------------- + + def test_non_marketplace_uses_git_flow(self): + """``apm view org/repo versions`` still uses the git-based path.""" + from apm_cli.models.dependency.types import GitReferenceType, RemoteRef + + mock_refs = [ + RemoteRef( + name="v1.0.0", + ref_type=GitReferenceType.TAG, + commit_sha="aabbccdd11223344", + ), + ] + with patch( + "apm_cli.commands.view.GitHubPackageDownloader" + ) as mock_cls, patch("apm_cli.commands.view.AuthResolver"): + mock_cls.return_value.list_remote_refs.return_value = mock_refs + with _force_rich_fallback(): + result = self.runner.invoke( + cli, ["view", "myorg/myrepo", "versions"] + ) + + assert result.exit_code == 0 + assert "v1.0.0" in result.output + assert "tag" in result.output + + # -- Non-semver versions are appended after semver entries ------------- + + def test_non_semver_versions_appended_at_end(self): + """Non-semver version strings appear after sorted semver entries.""" + plugin = _make_plugin( + "mixed-plugin", + versions=[ + ("nightly", "ref-nightly"), + ("2.0.0", "ref-200"), + ("1.0.0", "ref-100"), + ], + ) + manifest = _make_manifest([plugin]) + + with patch( + "apm_cli.marketplace.registry.get_marketplace_by_name", + return_value=_DUMMY_SOURCE, + ), patch( + "apm_cli.marketplace.client.fetch_or_cache", + return_value=manifest, + ): + with _force_rich_fallback(): + result = self.runner.invoke( + cli, ["view", "mixed-plugin@acme-tools", "versions"] + ) + + assert result.exit_code == 0 + output = result.output + # Semver entries before non-semver + pos_200 = output.index("2.0.0") + pos_100 = output.index("1.0.0") + pos_nightly = output.index("nightly") + assert pos_200 < pos_100 < pos_nightly + # "latest" only on 2.0.0 (semver), not "nightly" + assert result.output.count("latest") == 1 + + # -- Pin hint uses highest version ------------------------------------ + + def test_pin_hint_uses_highest_version(self): + """Install pin hint references the highest version.""" + plugin = _make_plugin( + "pin-plugin", + versions=[ + ("1.0.0", "ref-1"), + ("3.2.1", "ref-3"), + ], + ) + manifest = _make_manifest([plugin]) + + with patch( + "apm_cli.marketplace.registry.get_marketplace_by_name", + return_value=_DUMMY_SOURCE, + ), patch( + "apm_cli.marketplace.client.fetch_or_cache", + return_value=manifest, + ): + with _force_rich_fallback(): + result = self.runner.invoke( + cli, ["view", "pin-plugin@acme-tools", "versions"] + ) + + assert result.exit_code == 0 + assert "#^3.2.1" in result.output From 5acb808391b7c6881bcdf3d8837518ace61ee123 Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Sat, 11 Apr 2026 17:14:35 +0200 Subject: [PATCH 2/8] feat: marketplace publishing and validation tooling (Phase 2) Add 'apm marketplace publish' command that reads apm.yml defaults, resolves git HEAD SHA, validates semver, detects version conflicts, and updates marketplace.json with new version entries. Supports --dry-run, --force, and auto-detection of single marketplace. Add 'apm marketplace validate' command with validator.py module for schema validation, semver format checks, duplicate version detection, and duplicate name detection. Includes --check-refs placeholder and --verbose per-plugin details. 46 new tests (21 publish + 25 validate), 3955 total passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/commands/marketplace.py | 459 +++++++++++++ src/apm_cli/marketplace/validator.py | 137 ++++ .../marketplace/test_marketplace_publish.py | 643 ++++++++++++++++++ .../marketplace/test_marketplace_validator.py | 362 ++++++++++ 4 files changed, 1601 insertions(+) create mode 100644 src/apm_cli/marketplace/validator.py create mode 100644 tests/unit/marketplace/test_marketplace_publish.py create mode 100644 tests/unit/marketplace/test_marketplace_validator.py diff --git a/src/apm_cli/commands/marketplace.py b/src/apm_cli/commands/marketplace.py index ef4e201d..b7026e05 100644 --- a/src/apm_cli/commands/marketplace.py +++ b/src/apm_cli/commands/marketplace.py @@ -368,6 +368,96 @@ def remove(name, yes, verbose): sys.exit(1) +# --------------------------------------------------------------------------- +# marketplace validate +# --------------------------------------------------------------------------- + + +@marketplace.command(help="Validate a marketplace manifest") +@click.argument("name", required=True) +@click.option( + "--check-refs", is_flag=True, help="Verify version refs are reachable (network)" +) +@click.option("--verbose", "-v", is_flag=True, help="Show detailed output") +def validate(name, check_refs, verbose): + """Validate the manifest of a registered marketplace.""" + logger = CommandLogger("marketplace-validate", verbose=verbose) + try: + from ..marketplace.client import fetch_marketplace + from ..marketplace.registry import get_marketplace_by_name + from ..marketplace.validator import validate_marketplace + + source = get_marketplace_by_name(name) + logger.start(f"Validating marketplace '{name}'...", symbol="gear") + + manifest = fetch_marketplace(source, force_refresh=True) + + # Count version entries across all plugins + total_versions = sum(len(p.versions) for p in manifest.plugins) + logger.progress( + f"Found {len(manifest.plugins)} plugins, " + f"{total_versions} version entries", + symbol="info", + ) + + # Verbose: per-plugin details + if verbose: + for p in manifest.plugins: + source_type = "dict" if isinstance(p.source, dict) else "string" + logger.verbose_detail( + f" {p.name}: {len(p.versions)} versions, " + f"source type: {source_type}" + ) + + # Run validation + results = validate_marketplace(manifest) + + # Check-refs placeholder + if check_refs: + logger.warning( + "Ref checking not yet implemented -- skipping ref " + "reachability checks", + symbol="warning", + ) + + # Render results + passed = 0 + warning_count = 0 + error_count = 0 + click.echo() + click.echo("Validation Results:") + for r in results: + if r.passed and not r.warnings: + logger.success( + f" {r.check_name}: all plugins valid", symbol="check" + ) + passed += 1 + elif r.warnings and not r.errors: + for w in r.warnings: + logger.warning(f" {r.check_name}: {w}", symbol="warning") + warning_count += len(r.warnings) + else: + for e in r.errors: + logger.error(f" {r.check_name}: {e}", symbol="error") + for w in r.warnings: + logger.warning(f" {r.check_name}: {w}", symbol="warning") + error_count += len(r.errors) + warning_count += len(r.warnings) + + click.echo() + click.echo( + f"Summary: {passed} passed, {warning_count} warnings, " + f"{error_count} errors" + ) + + if error_count > 0: + sys.exit(1) + + except Exception as e: + logger.error(f"Failed to validate marketplace: {e}") + sys.exit(1) + + # --------------------------------------------------------------------------- # Top-level search command (registered separately in cli.py) # --------------------------------------------------------------------------- @@ -467,3 +557,372 @@ def search(expression, limit, verbose): except Exception as e: logger.error(f"Search failed: {e}") sys.exit(1) + + +# --------------------------------------------------------------------------- +# marketplace publish -- helpers +# --------------------------------------------------------------------------- + + +def _get_git_head_sha(): + """Get the current git HEAD commit SHA. + + Returns: + The full SHA string, or ``None`` if git is not available or the + working directory is not a git repository. + """ + import subprocess + + try: + result = subprocess.run( + ["git", "rev-parse", "HEAD"], + capture_output=True, + text=True, + encoding="utf-8", + timeout=5, + ) + if result.returncode == 0: + return result.stdout.strip() + except Exception: + pass + return None + + +def _find_local_marketplace_repo(source): + """Try to find a local clone of the marketplace repo. + + Checks whether the current working directory (or its git root) has a + remote matching *source*'s ``owner/repo``. + + Returns: + Repository root path as a string, or ``None``. + """ + import subprocess + + try: + # Get git repo root first + root_result = subprocess.run( + ["git", "rev-parse", "--show-toplevel"], + capture_output=True, + text=True, + encoding="utf-8", + timeout=5, + ) + if root_result.returncode != 0: + return None + + repo_root = root_result.stdout.strip() + + # Check all remotes for a match + result = subprocess.run( + ["git", "remote", "-v"], + capture_output=True, + text=True, + encoding="utf-8", + timeout=5, + cwd=repo_root, + ) + if result.returncode != 0: + return None + + expected = f"{source.owner}/{source.repo}" + for line in result.stdout.splitlines(): + if expected in line: + return repo_root + except Exception: + pass + return None + + +def _clone_marketplace_repo(source): + """Clone the marketplace repo to a temporary directory. + + Returns: + Path to the cloned directory. + + Raises: + RuntimeError: If cloning fails. + """ + import subprocess + import tempfile + + clone_url = f"https://{source.host}/{source.owner}/{source.repo}.git" + tmp_dir = tempfile.mkdtemp(prefix="apm-marketplace-") + try: + subprocess.run( + [ + "git", + "clone", + "--depth=1", + "--branch", + source.branch, + clone_url, + tmp_dir, + ], + check=True, + capture_output=True, + text=True, + encoding="utf-8", + timeout=60, + ) + except subprocess.CalledProcessError as exc: + raise RuntimeError( + f"Failed to clone marketplace repo " + f"'{source.owner}/{source.repo}': {exc.stderr.strip()}" + ) from exc + return tmp_dir + + +def _update_marketplace_file(file_path, plugin_name, version_str, ref, force): + """Read marketplace.json, add a version entry, and write back. + + Operates on the raw JSON dict so no fields are lost during + round-tripping through the frozen dataclass layer. + + Args: + file_path: Absolute path to marketplace.json. + plugin_name: Plugin to update (case-insensitive match). + version_str: Semver version string to publish. + ref: Git ref for the version entry. + force: When ``True``, overwrite an existing entry with the same + version string. + + Returns: + The *file_path* that was written. + + Raises: + FileNotFoundError: If *file_path* does not exist. + ValueError: If the plugin is not found in the file. + """ + import json + + with open(file_path, "r", encoding="utf-8") as fh: + data = json.load(fh) + + plugins = data.get("plugins", []) + target = None + for entry in plugins: + if entry.get("name", "").lower() == plugin_name.lower(): + target = entry + break + + if target is None: + raise ValueError( + f"Plugin '{plugin_name}' not found in {file_path}" + ) + + versions = target.get("versions", []) + + # Remove existing version entry when --force is active + if force: + versions = [v for v in versions if v.get("version") != version_str] + + versions.append({"version": version_str, "ref": ref}) + target["versions"] = versions + + with open(file_path, "w", encoding="utf-8") as fh: + json.dump(data, fh, indent=2) + fh.write("\n") + + return file_path + + +# --------------------------------------------------------------------------- +# marketplace publish +# --------------------------------------------------------------------------- + + +@marketplace.command(help="Publish a version entry to a marketplace") +@click.option( + "--marketplace", + "-m", + "marketplace_name", + default=None, + help="Target marketplace name", +) +@click.option( + "--version", + "version_str", + default=None, + help="Version to publish (semver X.Y.Z, default: from apm.yml)", +) +@click.option( + "--ref", + default=None, + help="Git ref / commit SHA (default: current HEAD)", +) +@click.option( + "--plugin", + "plugin_name", + default=None, + help="Plugin name in the marketplace (default: name from apm.yml)", +) +@click.option("--dry-run", is_flag=True, help="Show what would be published without making changes") +@click.option("--force", is_flag=True, help="Overwrite existing version entry with a different ref") +@click.option("--verbose", "-v", is_flag=True, help="Show detailed output") +def publish(marketplace_name, version_str, ref, plugin_name, dry_run, force, verbose): + """Publish a new version entry for a plugin in a marketplace.""" + logger = CommandLogger("marketplace-publish", verbose=verbose, dry_run=dry_run) + try: + import os + from pathlib import Path + + from ..marketplace.client import fetch_marketplace + from ..marketplace.registry import ( + get_marketplace_by_name, + get_registered_marketplaces, + ) + from ..marketplace.version_resolver import _parse_semver + from ..models.apm_package import APMPackage + + # -- 1. Resolve defaults from apm.yml --------------------------------- + if not plugin_name or not version_str: + apm_yml_path = Path("apm.yml") + if not apm_yml_path.exists(): + missing = [] + if not plugin_name: + missing.append("--plugin") + if not version_str: + missing.append("--version") + logger.error( + f"No apm.yml found. Specify {' and '.join(missing)} explicitly." + ) + sys.exit(1) + + package = APMPackage.from_apm_yml(apm_yml_path) + if not plugin_name: + plugin_name = package.name + if not version_str: + version_str = package.version + + # -- 2. Validate version ----------------------------------------------- + try: + _parse_semver(version_str) + except ValueError as exc: + logger.error(str(exc)) + sys.exit(1) + + # -- 3. Resolve git ref ------------------------------------------------ + if not ref: + ref = _get_git_head_sha() + if not ref: + logger.error( + "Could not determine git HEAD SHA. " + "Use --ref to specify a commit." + ) + sys.exit(1) + + # -- 4. Resolve marketplace -------------------------------------------- + if not marketplace_name: + sources = get_registered_marketplaces() + if len(sources) == 0: + logger.error( + "No marketplaces registered. " + "Use 'apm marketplace add OWNER/REPO' first." + ) + sys.exit(1) + elif len(sources) == 1: + marketplace_name = sources[0].name + logger.verbose_detail( + f" Auto-selected marketplace: {marketplace_name}" + ) + else: + names = ", ".join(s.name for s in sources) + logger.error( + f"Multiple marketplaces registered ({names}). " + f"Use --marketplace to specify which one." + ) + sys.exit(1) + + source = get_marketplace_by_name(marketplace_name) + + # -- 5. Start output --------------------------------------------------- + logger.start( + f"Publishing {plugin_name} v{version_str} " + f"to marketplace '{marketplace_name}'...", + symbol="gear", + ) + logger.progress(f"Ref: {ref}", symbol="info") + + # -- 6. Fetch manifest and validate ------------------------------------ + manifest = fetch_marketplace(source, force_refresh=True) + + plugin = manifest.find_plugin(plugin_name) + if plugin is None: + logger.error( + f"Plugin '{plugin_name}' not found in marketplace " + f"'{marketplace_name}'. " + f"Run 'apm marketplace browse {marketplace_name}' " + f"to see available plugins." + ) + sys.exit(1) + + # -- 7. Check for existing version ------------------------------------- + for existing in plugin.versions: + if existing.version == version_str: + if existing.ref == ref: + logger.progress( + f"Version {version_str} already published " + f"with same ref. Skipping.", + symbol="info", + ) + return + if not force: + logger.error( + f"Version {version_str} already exists with a " + f"different ref ({existing.ref}). " + f"Use --force to overwrite." + ) + sys.exit(1) + + # -- 8. Dry-run gate --------------------------------------------------- + if dry_run: + logger.dry_run_notice( + f"Would publish {plugin_name} v{version_str} " + f"(ref: {ref}) to '{marketplace_name}'" + ) + return + + # -- 9. Locate or clone marketplace repo and update -------------------- + local_repo = _find_local_marketplace_repo(source) + cloned = False + if local_repo is None: + logger.verbose_detail( + " Marketplace repo not found locally, cloning..." + ) + local_repo = _clone_marketplace_repo(source) + cloned = True + + marketplace_file = os.path.join(local_repo, source.path) + + if not os.path.isfile(marketplace_file): + logger.error( + f"marketplace.json not found at expected path: " + f"{marketplace_file}" + ) + sys.exit(1) + + _update_marketplace_file( + marketplace_file, plugin_name, version_str, ref, force + ) + + logger.success( + "Version entry added to marketplace.json", symbol="check" + ) + logger.progress( + f"Marketplace file: {marketplace_file}", symbol="info" + ) + logger.progress( + "Don't forget to commit and push the marketplace repo!", + symbol="info", + ) + if cloned: + logger.progress( + f"Cloned repo location: {local_repo}", symbol="info" + ) + + except SystemExit: + raise + except Exception as e: + logger.error(f"Failed to publish: {e}") + sys.exit(1) diff --git a/src/apm_cli/marketplace/validator.py b/src/apm_cli/marketplace/validator.py new file mode 100644 index 00000000..cf8f0af2 --- /dev/null +++ b/src/apm_cli/marketplace/validator.py @@ -0,0 +1,137 @@ +"""Marketplace manifest validation. + +Provides validation functions for marketplace.json integrity checking. +Used by ``apm marketplace validate`` and potentially by ``apm marketplace publish``. + +All validators operate on parsed ``MarketplaceManifest`` / ``MarketplacePlugin`` +objects. The JSON parser (``models.py``) already drops entries that are +structurally unrecognizable; these validators enforce additional business +rules on the successfully parsed entries. +""" + +import re +from dataclasses import dataclass, field +from typing import List, Sequence + +from .models import MarketplaceManifest, MarketplacePlugin + +# Strict semver: X.Y.Z with integer components only (matches version_resolver). +_SEMVER_RE = re.compile(r"^(\d+)\.(\d+)\.(\d+)$") + + +@dataclass +class ValidationResult: + """Result of a single validation check.""" + + check_name: str + passed: bool + warnings: List[str] = field(default_factory=list) + errors: List[str] = field(default_factory=list) + + +def validate_marketplace( + manifest: MarketplaceManifest, +) -> List[ValidationResult]: + """Run all validation checks on a marketplace manifest. + + Returns a list of ``ValidationResult`` objects, one per check. + """ + plugins = manifest.plugins + return [ + validate_plugin_schema(plugins), + validate_version_format(plugins), + validate_no_duplicate_versions(plugins), + validate_no_duplicate_names(plugins), + ] + + +def validate_plugin_schema( + plugins: Sequence[MarketplacePlugin], +) -> ValidationResult: + """Check all plugins have required fields (name, source).""" + errors: List[str] = [] + for plugin in plugins: + if not plugin.name or not plugin.name.strip(): + errors.append("Plugin entry has empty name") + if plugin.source is None: + errors.append( + f"Plugin '{plugin.name}' is missing required field 'source'" + ) + return ValidationResult( + check_name="Schema", + passed=len(errors) == 0, + errors=errors, + ) + + +def validate_version_format( + plugins: Sequence[MarketplacePlugin], +) -> ValidationResult: + """Check all version entries have valid semver and non-empty ref.""" + warnings: List[str] = [] + errors: List[str] = [] + for plugin in plugins: + for entry in plugin.versions: + ver = entry.version.strip() if entry.version else "" + ref = entry.ref.strip() if entry.ref else "" + if ver and not _SEMVER_RE.match(ver): + warnings.append( + f"Plugin '{plugin.name}' version '{entry.version}' " + f"is not valid semver (expected X.Y.Z)" + ) + if not ref: + errors.append( + f"Plugin '{plugin.name}' version '{entry.version}' " + f"has empty ref" + ) + return ValidationResult( + check_name="Versions", + passed=len(errors) == 0 and len(warnings) == 0, + warnings=warnings, + errors=errors, + ) + + +def validate_no_duplicate_versions( + plugins: Sequence[MarketplacePlugin], +) -> ValidationResult: + """Check no plugin has duplicate version strings.""" + warnings: List[str] = [] + for plugin in plugins: + seen: dict = {} + for entry in plugin.versions: + normalized = entry.version.strip() + if normalized in seen: + warnings.append( + f"Plugin '{plugin.name}' has duplicate version " + f"'{normalized}'" + ) + else: + seen[normalized] = True + return ValidationResult( + check_name="Duplicate versions", + passed=len(warnings) == 0, + warnings=warnings, + ) + + +def validate_no_duplicate_names( + plugins: Sequence[MarketplacePlugin], +) -> ValidationResult: + """Check no two plugins share the same name (case-insensitive).""" + errors: List[str] = [] + seen: dict = {} + for plugin in plugins: + lower = plugin.name.strip().lower() + if lower in seen: + errors.append( + f"Duplicate plugin name: '{plugin.name}' " + f"(conflicts with '{seen[lower]}')" + ) + else: + seen[lower] = plugin.name + return ValidationResult( + check_name="Names", + passed=len(errors) == 0, + errors=errors, + ) diff --git a/tests/unit/marketplace/test_marketplace_publish.py b/tests/unit/marketplace/test_marketplace_publish.py new file mode 100644 index 00000000..ab1137f7 --- /dev/null +++ b/tests/unit/marketplace/test_marketplace_publish.py @@ -0,0 +1,643 @@ +"""Tests for ``apm marketplace publish`` command.""" + +import json +import os +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest +from click.testing import CliRunner + +from apm_cli.marketplace.models import ( + MarketplaceManifest, + MarketplacePlugin, + MarketplaceSource, + VersionEntry, +) + + +@pytest.fixture +def runner(): + return CliRunner() + + +@pytest.fixture(autouse=True) +def _isolate_config(tmp_path, monkeypatch): + """Isolate filesystem writes so tests never touch real config.""" + config_dir = str(tmp_path / ".apm") + monkeypatch.setattr("apm_cli.config.CONFIG_DIR", config_dir) + monkeypatch.setattr( + "apm_cli.config.CONFIG_FILE", str(tmp_path / ".apm" / "config.json") + ) + monkeypatch.setattr("apm_cli.config._config_cache", None) + monkeypatch.setattr("apm_cli.marketplace.registry._registry_cache", None) + + +# --------------------------------------------------------------------------- +# Shared fixtures / helpers +# --------------------------------------------------------------------------- + +_ACME_SOURCE = MarketplaceSource( + name="acme-tools", + owner="acme-org", + repo="marketplace", +) + +_MANIFEST_WITH_PLUGIN = MarketplaceManifest( + name="acme-tools", + plugins=( + MarketplacePlugin( + name="skill-auth", + source={"type": "github", "repo": "acme-org/skill-auth"}, + description="Auth skill", + version="2.0.0", + versions=( + VersionEntry(version="1.0.0", ref="aaa111"), + VersionEntry(version="2.0.0", ref="bbb222"), + ), + ), + ), +) + +_MANIFEST_NO_VERSIONS = MarketplaceManifest( + name="acme-tools", + plugins=( + MarketplacePlugin( + name="skill-auth", + source={"type": "github", "repo": "acme-org/skill-auth"}, + description="Auth skill", + ), + ), +) + + +def _make_mock_package(name="skill-auth", version="3.0.0"): + pkg = MagicMock() + pkg.name = name + pkg.version = version + return pkg + + +def _make_marketplace_json(tmp_path, plugins=None): + """Write a minimal marketplace.json and return its path.""" + if plugins is None: + plugins = [ + { + "name": "skill-auth", + "source": {"type": "github", "repo": "acme-org/skill-auth"}, + "description": "Auth skill", + "version": "2.0.0", + "versions": [ + {"version": "1.0.0", "ref": "aaa111"}, + {"version": "2.0.0", "ref": "bbb222"}, + ], + } + ] + data = {"name": "acme-tools", "plugins": plugins} + mp_file = tmp_path / "marketplace.json" + mp_file.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8") + return str(mp_file) + + +# --------------------------------------------------------------------------- +# Tests +# --------------------------------------------------------------------------- + + +class TestPublishDefaults: + """Publish with all defaults -- reads apm.yml + git HEAD.""" + + @patch("apm_cli.commands.marketplace._update_marketplace_file") + @patch("apm_cli.commands.marketplace._find_local_marketplace_repo") + @patch("apm_cli.marketplace.client.fetch_marketplace") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + @patch("apm_cli.marketplace.registry.get_registered_marketplaces") + @patch("apm_cli.commands.marketplace._get_git_head_sha") + @patch("apm_cli.models.apm_package.APMPackage.from_apm_yml") + def test_publish_all_defaults( + self, + mock_from_apm, + mock_git_sha, + mock_get_all, + mock_get_by_name, + mock_fetch, + mock_find_local, + mock_update_file, + runner, + tmp_path, + ): + from apm_cli.commands.marketplace import marketplace + + mock_from_apm.return_value = _make_mock_package() + mock_git_sha.return_value = "d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0c1d2e3" + mock_get_all.return_value = [_ACME_SOURCE] + mock_get_by_name.return_value = _ACME_SOURCE + mock_fetch.return_value = _MANIFEST_NO_VERSIONS + mock_find_local.return_value = str(tmp_path) + mock_update_file.return_value = str(tmp_path / "marketplace.json") + + # Create apm.yml and a placeholder marketplace.json + with runner.isolated_filesystem(temp_dir=tmp_path): + Path("apm.yml").write_text("name: skill-auth\nversion: 3.0.0\n") + _make_marketplace_json(tmp_path) + result = runner.invoke(marketplace, ["publish"]) + + assert result.exit_code == 0, result.output + assert "skill-auth" in result.output + assert "v3.0.0" in result.output + mock_update_file.assert_called_once_with( + str(tmp_path / "marketplace.json"), + "skill-auth", + "3.0.0", + "d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0c1d2e3", + False, + ) + + +class TestPublishExplicitArgs: + """Publish with explicit --version, --ref, --marketplace, --plugin.""" + + @patch("apm_cli.commands.marketplace._update_marketplace_file") + @patch("apm_cli.commands.marketplace._find_local_marketplace_repo") + @patch("apm_cli.marketplace.client.fetch_marketplace") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_publish_explicit_options( + self, + mock_get_by_name, + mock_fetch, + mock_find_local, + mock_update_file, + runner, + tmp_path, + ): + from apm_cli.commands.marketplace import marketplace + + mock_get_by_name.return_value = _ACME_SOURCE + mock_fetch.return_value = _MANIFEST_NO_VERSIONS + mock_find_local.return_value = str(tmp_path) + mock_update_file.return_value = str(tmp_path / "marketplace.json") + + _make_marketplace_json(tmp_path) + + result = runner.invoke( + marketplace, + [ + "publish", + "--marketplace", + "acme-tools", + "--version", + "4.0.0", + "--ref", + "abc123def456", + "--plugin", + "skill-auth", + ], + ) + + assert result.exit_code == 0, result.output + assert "v4.0.0" in result.output + mock_update_file.assert_called_once_with( + str(tmp_path / "marketplace.json"), + "skill-auth", + "4.0.0", + "abc123def456", + False, + ) + + +class TestPublishVersionConflict: + """Version already exists -- same ref (skip) or different ref (error).""" + + @patch("apm_cli.marketplace.client.fetch_marketplace") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_same_version_same_ref_skips( + self, mock_get_by_name, mock_fetch, runner + ): + from apm_cli.commands.marketplace import marketplace + + mock_get_by_name.return_value = _ACME_SOURCE + mock_fetch.return_value = _MANIFEST_WITH_PLUGIN + + result = runner.invoke( + marketplace, + [ + "publish", + "--marketplace", + "acme-tools", + "--version", + "2.0.0", + "--ref", + "bbb222", + "--plugin", + "skill-auth", + ], + ) + + assert result.exit_code == 0, result.output + assert "already published" in result.output.lower() + + @patch("apm_cli.marketplace.client.fetch_marketplace") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_same_version_different_ref_errors( + self, mock_get_by_name, mock_fetch, runner + ): + from apm_cli.commands.marketplace import marketplace + + mock_get_by_name.return_value = _ACME_SOURCE + mock_fetch.return_value = _MANIFEST_WITH_PLUGIN + + result = runner.invoke( + marketplace, + [ + "publish", + "--marketplace", + "acme-tools", + "--version", + "2.0.0", + "--ref", + "different_sha", + "--plugin", + "skill-auth", + ], + ) + + assert result.exit_code != 0 + assert "already exists" in result.output.lower() + assert "--force" in result.output + + +class TestPublishForce: + """--force overwrites existing version entry with different ref.""" + + @patch("apm_cli.commands.marketplace._update_marketplace_file") + @patch("apm_cli.commands.marketplace._find_local_marketplace_repo") + @patch("apm_cli.marketplace.client.fetch_marketplace") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_force_overwrites( + self, + mock_get_by_name, + mock_fetch, + mock_find_local, + mock_update_file, + runner, + tmp_path, + ): + from apm_cli.commands.marketplace import marketplace + + mock_get_by_name.return_value = _ACME_SOURCE + mock_fetch.return_value = _MANIFEST_WITH_PLUGIN + mock_find_local.return_value = str(tmp_path) + mock_update_file.return_value = str(tmp_path / "marketplace.json") + + _make_marketplace_json(tmp_path) + + result = runner.invoke( + marketplace, + [ + "publish", + "--marketplace", + "acme-tools", + "--version", + "2.0.0", + "--ref", + "new_sha_999", + "--plugin", + "skill-auth", + "--force", + ], + ) + + assert result.exit_code == 0, result.output + mock_update_file.assert_called_once_with( + str(tmp_path / "marketplace.json"), + "skill-auth", + "2.0.0", + "new_sha_999", + True, + ) + + +class TestPublishDryRun: + """--dry-run shows what would be published without writing.""" + + @patch("apm_cli.commands.marketplace._update_marketplace_file") + @patch("apm_cli.marketplace.client.fetch_marketplace") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_dry_run_no_writes( + self, + mock_get_by_name, + mock_fetch, + mock_update_file, + runner, + ): + from apm_cli.commands.marketplace import marketplace + + mock_get_by_name.return_value = _ACME_SOURCE + mock_fetch.return_value = _MANIFEST_NO_VERSIONS + + result = runner.invoke( + marketplace, + [ + "publish", + "--marketplace", + "acme-tools", + "--version", + "5.0.0", + "--ref", + "abc123", + "--plugin", + "skill-auth", + "--dry-run", + ], + ) + + assert result.exit_code == 0, result.output + assert "dry-run" in result.output.lower() + mock_update_file.assert_not_called() + + +class TestPublishErrorCases: + """Error paths: marketplace not registered, plugin missing, bad semver, etc.""" + + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_marketplace_not_registered(self, mock_get_by_name, runner): + from apm_cli.commands.marketplace import marketplace + from apm_cli.marketplace.errors import MarketplaceNotFoundError + + mock_get_by_name.side_effect = MarketplaceNotFoundError("ghost") + + result = runner.invoke( + marketplace, + [ + "publish", + "--marketplace", + "ghost", + "--version", + "1.0.0", + "--ref", + "abc", + "--plugin", + "some-plugin", + ], + ) + + assert result.exit_code != 0 + assert "ghost" in result.output.lower() + + @patch("apm_cli.marketplace.client.fetch_marketplace") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_plugin_not_found(self, mock_get_by_name, mock_fetch, runner): + from apm_cli.commands.marketplace import marketplace + + mock_get_by_name.return_value = _ACME_SOURCE + mock_fetch.return_value = MarketplaceManifest( + name="acme-tools", plugins=() + ) + + result = runner.invoke( + marketplace, + [ + "publish", + "--marketplace", + "acme-tools", + "--version", + "1.0.0", + "--ref", + "abc", + "--plugin", + "nonexistent-plugin", + ], + ) + + assert result.exit_code != 0 + assert "not found" in result.output.lower() + + def test_invalid_semver_version(self, runner): + from apm_cli.commands.marketplace import marketplace + + result = runner.invoke( + marketplace, + [ + "publish", + "--marketplace", + "acme-tools", + "--version", + "not-a-version", + "--ref", + "abc", + "--plugin", + "skill-auth", + ], + ) + + assert result.exit_code != 0 + assert "invalid" in result.output.lower() or "semver" in result.output.lower() + + def test_no_apm_yml_and_no_flags(self, runner, tmp_path): + from apm_cli.commands.marketplace import marketplace + + with runner.isolated_filesystem(temp_dir=tmp_path): + result = runner.invoke( + marketplace, + [ + "publish", + "--marketplace", + "acme-tools", + "--ref", + "abc123", + ], + ) + + assert result.exit_code != 0 + assert "apm.yml" in result.output.lower() or "--plugin" in result.output + + @patch("apm_cli.marketplace.registry.get_registered_marketplaces") + def test_no_marketplaces_registered(self, mock_get_all, runner): + from apm_cli.commands.marketplace import marketplace + + mock_get_all.return_value = [] + + result = runner.invoke( + marketplace, + [ + "publish", + "--version", + "1.0.0", + "--ref", + "abc", + "--plugin", + "skill-auth", + ], + ) + + assert result.exit_code != 0 + assert "no marketplace" in result.output.lower() + + @patch("apm_cli.marketplace.registry.get_registered_marketplaces") + def test_multiple_marketplaces_without_flag(self, mock_get_all, runner): + from apm_cli.commands.marketplace import marketplace + + mock_get_all.return_value = [ + MarketplaceSource(name="m1", owner="a", repo="b"), + MarketplaceSource(name="m2", owner="c", repo="d"), + ] + + result = runner.invoke( + marketplace, + [ + "publish", + "--version", + "1.0.0", + "--ref", + "abc", + "--plugin", + "skill-auth", + ], + ) + + assert result.exit_code != 0 + assert "multiple" in result.output.lower() or "--marketplace" in result.output + + +class TestPublishAutoDetect: + """Auto-detect marketplace when only one is registered.""" + + @patch("apm_cli.commands.marketplace._update_marketplace_file") + @patch("apm_cli.commands.marketplace._find_local_marketplace_repo") + @patch("apm_cli.marketplace.client.fetch_marketplace") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + @patch("apm_cli.marketplace.registry.get_registered_marketplaces") + def test_auto_selects_single_marketplace( + self, + mock_get_all, + mock_get_by_name, + mock_fetch, + mock_find_local, + mock_update_file, + runner, + tmp_path, + ): + from apm_cli.commands.marketplace import marketplace + + mock_get_all.return_value = [_ACME_SOURCE] + mock_get_by_name.return_value = _ACME_SOURCE + mock_fetch.return_value = _MANIFEST_NO_VERSIONS + mock_find_local.return_value = str(tmp_path) + mock_update_file.return_value = str(tmp_path / "marketplace.json") + + _make_marketplace_json(tmp_path) + + result = runner.invoke( + marketplace, + [ + "publish", + "--version", + "1.0.0", + "--ref", + "sha123", + "--plugin", + "skill-auth", + ], + ) + + assert result.exit_code == 0, result.output + # Should have resolved to acme-tools + mock_get_by_name.assert_called_once_with("acme-tools") + + +# --------------------------------------------------------------------------- +# _update_marketplace_file unit tests (real file I/O on tmp_path) +# --------------------------------------------------------------------------- + + +class TestUpdateMarketplaceFile: + """Integration-style tests for the raw JSON read/modify/write helper.""" + + def test_adds_new_version_entry(self, tmp_path): + from apm_cli.commands.marketplace import _update_marketplace_file + + mp_file = _make_marketplace_json(tmp_path) + _update_marketplace_file(mp_file, "skill-auth", "3.0.0", "ccc333", False) + + data = json.loads(Path(mp_file).read_text(encoding="utf-8")) + versions = data["plugins"][0]["versions"] + assert len(versions) == 3 + assert versions[-1] == {"version": "3.0.0", "ref": "ccc333"} + + def test_force_replaces_existing_version(self, tmp_path): + from apm_cli.commands.marketplace import _update_marketplace_file + + mp_file = _make_marketplace_json(tmp_path) + _update_marketplace_file(mp_file, "skill-auth", "2.0.0", "new_ref", True) + + data = json.loads(Path(mp_file).read_text(encoding="utf-8")) + versions = data["plugins"][0]["versions"] + # Old 2.0.0 replaced, 1.0.0 still present + ver_map = {v["version"]: v["ref"] for v in versions} + assert ver_map["2.0.0"] == "new_ref" + assert ver_map["1.0.0"] == "aaa111" + assert len(versions) == 2 + + def test_raises_on_missing_plugin(self, tmp_path): + from apm_cli.commands.marketplace import _update_marketplace_file + + mp_file = _make_marketplace_json(tmp_path) + with pytest.raises(ValueError, match="not found"): + _update_marketplace_file(mp_file, "nonexistent", "1.0.0", "ref", False) + + def test_case_insensitive_plugin_match(self, tmp_path): + from apm_cli.commands.marketplace import _update_marketplace_file + + mp_file = _make_marketplace_json(tmp_path) + _update_marketplace_file(mp_file, "SKILL-AUTH", "3.0.0", "ccc333", False) + + data = json.loads(Path(mp_file).read_text(encoding="utf-8")) + versions = data["plugins"][0]["versions"] + assert len(versions) == 3 + + def test_creates_versions_array_when_missing(self, tmp_path): + from apm_cli.commands.marketplace import _update_marketplace_file + + plugins = [ + { + "name": "bare-plugin", + "source": {"type": "github", "repo": "acme-org/bare"}, + } + ] + mp_file = _make_marketplace_json(tmp_path, plugins=plugins) + _update_marketplace_file(mp_file, "bare-plugin", "1.0.0", "sha1", False) + + data = json.loads(Path(mp_file).read_text(encoding="utf-8")) + versions = data["plugins"][0]["versions"] + assert versions == [{"version": "1.0.0", "ref": "sha1"}] + + +# --------------------------------------------------------------------------- +# _get_git_head_sha unit tests +# --------------------------------------------------------------------------- + + +class TestGetGitHeadSha: + """Tests for the git HEAD SHA helper.""" + + @patch("subprocess.run") + def test_returns_sha(self, mock_run): + from apm_cli.commands.marketplace import _get_git_head_sha + + mock_run.return_value = MagicMock( + returncode=0, stdout="abc123def456\n" + ) + assert _get_git_head_sha() == "abc123def456" + + @patch("subprocess.run") + def test_returns_none_on_failure(self, mock_run): + from apm_cli.commands.marketplace import _get_git_head_sha + + mock_run.return_value = MagicMock(returncode=128, stdout="") + assert _get_git_head_sha() is None + + @patch("subprocess.run", side_effect=OSError("no git")) + def test_returns_none_on_exception(self, mock_run): + from apm_cli.commands.marketplace import _get_git_head_sha + + assert _get_git_head_sha() is None diff --git a/tests/unit/marketplace/test_marketplace_validator.py b/tests/unit/marketplace/test_marketplace_validator.py new file mode 100644 index 00000000..20403bad --- /dev/null +++ b/tests/unit/marketplace/test_marketplace_validator.py @@ -0,0 +1,362 @@ +"""Tests for marketplace manifest validator and validate CLI command.""" + +from unittest.mock import patch + +import pytest +from click.testing import CliRunner + +from apm_cli.marketplace.models import ( + MarketplaceManifest, + MarketplacePlugin, + MarketplaceSource, + VersionEntry, +) +from apm_cli.marketplace.validator import ( + ValidationResult, + validate_marketplace, + validate_no_duplicate_names, + validate_no_duplicate_versions, + validate_plugin_schema, + validate_version_format, +) + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture +def runner(): + return CliRunner() + + +@pytest.fixture(autouse=True) +def _isolate_config(tmp_path, monkeypatch): + """Isolate filesystem writes (mirrors test_marketplace_commands.py).""" + config_dir = str(tmp_path / ".apm") + monkeypatch.setattr("apm_cli.config.CONFIG_DIR", config_dir) + monkeypatch.setattr( + "apm_cli.config.CONFIG_FILE", str(tmp_path / ".apm" / "config.json") + ) + monkeypatch.setattr("apm_cli.config._config_cache", None) + monkeypatch.setattr( + "apm_cli.marketplace.registry._registry_cache", None + ) + + +# --------------------------------------------------------------------------- +# Helper builders +# --------------------------------------------------------------------------- + + +def _plugin(name="test-plugin", source="owner/repo", versions=()): + """Convenience builder for a MarketplacePlugin.""" + return MarketplacePlugin( + name=name, + source=source, + versions=tuple(versions), + ) + + +def _version(ver="1.0.0", ref="abc123"): + """Convenience builder for a VersionEntry.""" + return VersionEntry(version=ver, ref=ref) + + +def _manifest(*plugins, name="test-marketplace"): + """Convenience builder for a MarketplaceManifest.""" + return MarketplaceManifest(name=name, plugins=tuple(plugins)) + + +# =================================================================== +# Unit tests -- validate_plugin_schema +# =================================================================== + + +class TestValidatePluginSchema: + """validate_plugin_schema checks name + source are present.""" + + def test_valid_plugins_pass(self): + plugins = [_plugin("a", "owner/a"), _plugin("b", "owner/b")] + result = validate_plugin_schema(plugins) + assert result.passed is True + assert result.errors == [] + + def test_plugin_missing_name(self): + plugins = [_plugin(name="", source="owner/repo")] + result = validate_plugin_schema(plugins) + assert result.passed is False + assert any("empty name" in e for e in result.errors) + + def test_plugin_missing_source(self): + plugins = [MarketplacePlugin(name="orphan", source=None)] + result = validate_plugin_schema(plugins) + assert result.passed is False + assert any("source" in e.lower() for e in result.errors) + + def test_empty_list_passes(self): + result = validate_plugin_schema([]) + assert result.passed is True + + +# =================================================================== +# Unit tests -- validate_version_format +# =================================================================== + + +class TestValidateVersionFormat: + """validate_version_format checks semver + non-empty ref.""" + + def test_valid_versions_pass(self): + plugins = [ + _plugin( + versions=[_version("1.0.0", "abc"), _version("2.3.4", "def")] + ) + ] + result = validate_version_format(plugins) + assert result.passed is True + assert result.warnings == [] + assert result.errors == [] + + def test_invalid_semver_warns(self): + plugins = [_plugin(versions=[_version("not-semver", "abc123")])] + result = validate_version_format(plugins) + assert result.passed is False + assert len(result.warnings) == 1 + assert "not valid semver" in result.warnings[0] + + def test_empty_ref_errors(self): + plugins = [_plugin(versions=[_version("1.0.0", "")])] + result = validate_version_format(plugins) + assert result.passed is False + assert len(result.errors) == 1 + assert "empty ref" in result.errors[0] + + def test_whitespace_only_ref_errors(self): + plugins = [_plugin(versions=[_version("1.0.0", " ")])] + result = validate_version_format(plugins) + assert result.passed is False + assert any("empty ref" in e for e in result.errors) + + def test_plugin_with_no_versions_passes(self): + plugins = [_plugin(versions=[])] + result = validate_version_format(plugins) + assert result.passed is True + + +# =================================================================== +# Unit tests -- validate_no_duplicate_versions +# =================================================================== + + +class TestValidateNoDuplicateVersions: + """validate_no_duplicate_versions checks per-plugin uniqueness.""" + + def test_unique_versions_pass(self): + plugins = [ + _plugin(versions=[_version("1.0.0"), _version("2.0.0")]) + ] + result = validate_no_duplicate_versions(plugins) + assert result.passed is True + assert result.warnings == [] + + def test_duplicate_version_warns(self): + plugins = [ + _plugin( + name="code-review", + versions=[_version("1.0.0", "aaa"), _version("1.0.0", "bbb")], + ) + ] + result = validate_no_duplicate_versions(plugins) + assert result.passed is False + assert len(result.warnings) == 1 + assert "code-review" in result.warnings[0] + assert "1.0.0" in result.warnings[0] + + def test_same_version_across_plugins_is_ok(self): + plugins = [ + _plugin(name="a", versions=[_version("1.0.0")]), + _plugin(name="b", versions=[_version("1.0.0")]), + ] + result = validate_no_duplicate_versions(plugins) + assert result.passed is True + + def test_empty_versions_pass(self): + plugins = [_plugin(versions=[])] + result = validate_no_duplicate_versions(plugins) + assert result.passed is True + + +# =================================================================== +# Unit tests -- validate_no_duplicate_names +# =================================================================== + + +class TestValidateNoDuplicateNames: + """validate_no_duplicate_names is case-insensitive.""" + + def test_unique_names_pass(self): + plugins = [_plugin(name="alpha"), _plugin(name="beta")] + result = validate_no_duplicate_names(plugins) + assert result.passed is True + assert result.errors == [] + + def test_duplicate_names_case_insensitive(self): + plugins = [_plugin(name="MyPlugin"), _plugin(name="myplugin")] + result = validate_no_duplicate_names(plugins) + assert result.passed is False + assert len(result.errors) == 1 + assert "myplugin" in result.errors[0].lower() + + def test_empty_list_passes(self): + result = validate_no_duplicate_names([]) + assert result.passed is True + + +# =================================================================== +# Unit tests -- validate_marketplace (integration of all checks) +# =================================================================== + + +class TestValidateMarketplace: + """validate_marketplace returns all check results.""" + + def test_valid_marketplace_returns_all_passed(self): + manifest = _manifest( + _plugin("a", "owner/a", versions=[_version("1.0.0")]), + _plugin("b", "owner/b", versions=[_version("2.0.0")]), + ) + results = validate_marketplace(manifest) + assert len(results) == 4 + assert all(r.passed for r in results) + + def test_empty_marketplace_passes_all(self): + manifest = _manifest() + results = validate_marketplace(manifest) + assert len(results) == 4 + assert all(r.passed for r in results) + + def test_returns_mixed_results(self): + manifest = _manifest( + _plugin( + name="good", + source="owner/good", + versions=[_version("1.0.0"), _version("1.0.0")], + ), + ) + results = validate_marketplace(manifest) + # Schema and Names should pass; Duplicate versions should fail + names_by_pass = {r.check_name: r.passed for r in results} + assert names_by_pass["Schema"] is True + assert names_by_pass["Names"] is True + assert names_by_pass["Duplicate versions"] is False + + +# =================================================================== +# CLI command tests -- apm marketplace validate +# =================================================================== + + +class TestValidateCommand: + """CLI command output and behavior.""" + + @patch("apm_cli.marketplace.client.fetch_marketplace") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_output_format(self, mock_get, mock_fetch, runner): + from apm_cli.commands.marketplace import marketplace + + mock_get.return_value = MarketplaceSource( + name="acme", owner="acme-org", repo="plugins" + ) + mock_fetch.return_value = _manifest( + _plugin("a", "owner/a", versions=[_version("1.0.0")]), + _plugin("b", "owner/b", versions=[_version("2.0.0")]), + ) + result = runner.invoke(marketplace, ["validate", "acme"]) + assert result.exit_code == 0 + assert "Validating marketplace" in result.output + assert "Validation Results:" in result.output + assert "Summary:" in result.output + assert "passed" in result.output + + @patch("apm_cli.marketplace.client.fetch_marketplace") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_verbose_shows_per_plugin_details( + self, mock_get, mock_fetch, runner + ): + from apm_cli.commands.marketplace import marketplace + + mock_get.return_value = MarketplaceSource( + name="acme", owner="acme-org", repo="plugins" + ) + mock_fetch.return_value = _manifest( + _plugin("alpha", "owner/alpha", versions=[_version("1.0.0")]), + ) + result = runner.invoke( + marketplace, ["validate", "acme", "--verbose"] + ) + assert result.exit_code == 0 + assert "alpha" in result.output + assert "1 versions" in result.output or "source type" in result.output + + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_unregistered_marketplace_errors(self, mock_get, runner): + from apm_cli.marketplace.errors import MarketplaceNotFoundError + from apm_cli.commands.marketplace import marketplace + + mock_get.side_effect = MarketplaceNotFoundError("nope") + result = runner.invoke(marketplace, ["validate", "nope"]) + assert result.exit_code != 0 + + @patch("apm_cli.marketplace.client.fetch_marketplace") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_check_refs_shows_warning(self, mock_get, mock_fetch, runner): + from apm_cli.commands.marketplace import marketplace + + mock_get.return_value = MarketplaceSource( + name="acme", owner="acme-org", repo="plugins" + ) + mock_fetch.return_value = _manifest( + _plugin("a", "owner/a"), + ) + result = runner.invoke( + marketplace, ["validate", "acme", "--check-refs"] + ) + assert result.exit_code == 0 + assert "not yet implemented" in result.output.lower() + + @patch("apm_cli.marketplace.client.fetch_marketplace") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_validation_errors_cause_nonzero_exit( + self, mock_get, mock_fetch, runner + ): + from apm_cli.commands.marketplace import marketplace + + mock_get.return_value = MarketplaceSource( + name="acme", owner="acme-org", repo="plugins" + ) + # Plugin with empty ref triggers an error + mock_fetch.return_value = _manifest( + _plugin("bad", "owner/bad", versions=[_version("1.0.0", "")]), + ) + result = runner.invoke(marketplace, ["validate", "acme"]) + assert result.exit_code != 0 + assert "error" in result.output.lower() + + @patch("apm_cli.marketplace.client.fetch_marketplace") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_plugin_count_in_output(self, mock_get, mock_fetch, runner): + from apm_cli.commands.marketplace import marketplace + + mock_get.return_value = MarketplaceSource( + name="acme", owner="acme-org", repo="plugins" + ) + mock_fetch.return_value = _manifest( + _plugin("a", "o/a"), + _plugin("b", "o/b"), + _plugin("c", "o/c"), + ) + result = runner.invoke(marketplace, ["validate", "acme"]) + assert result.exit_code == 0 + assert "3 plugins" in result.output From e03100a5684e38c88c115dc52186eead8e05ac94 Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Sat, 11 Apr 2026 17:28:21 +0200 Subject: [PATCH 3/8] feat: security hardening - version immutability and shadow detection (Phase 3) Add version pin cache (~/.apm/cache/marketplace/version-pins.json) that tracks version-to-ref mappings per plugin per marketplace. Warns when a previously pinned version's ref changes (possible ref swap attack). Advisory only -- never blocks installation. Add multi-marketplace shadow detection that checks all registered marketplaces for duplicate plugin names during resolution. Warns about potential name squatting when the same plugin exists in multiple marketplaces. Add security-critical provenance comments in install.py confirming discovered_via and marketplace_plugin_name are set for all marketplace dependencies. 35 new tests (25 immutability + 10 shadow), 3990 total passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/commands/install.py | 9 + src/apm_cli/marketplace/resolver.py | 42 ++ src/apm_cli/marketplace/shadow_detector.py | 75 ++++ src/apm_cli/marketplace/version_pins.py | 150 +++++++ .../unit/marketplace/test_shadow_detector.py | 312 +++++++++++++++ tests/unit/marketplace/test_version_pins.py | 371 ++++++++++++++++++ 6 files changed, 959 insertions(+) create mode 100644 src/apm_cli/marketplace/shadow_detector.py create mode 100644 src/apm_cli/marketplace/version_pins.py create mode 100644 tests/unit/marketplace/test_shadow_detector.py create mode 100644 tests/unit/marketplace/test_version_pins.py diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 92098c6b..de20c5c2 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -166,6 +166,10 @@ def _validate_and_add_packages_to_apm_yml(packages, dry_run=False, dev=False, lo logger.verbose_detail( f" Resolved to: {canonical_str}" ) + # Security-critical: record marketplace provenance so + # the lockfile tracks where each dependency was + # discovered. These fields enable supply-chain audits + # and prevent silent marketplace source confusion. marketplace_provenance = { "discovered_via": marketplace_name, "marketplace_plugin_name": plugin_name, @@ -2618,6 +2622,11 @@ def _collect_descendants(node, visited=None): if dep_key in _package_hashes: locked_dep.content_hash = _package_hashes[dep_key] # Attach marketplace provenance if available + # Security-critical: discovered_via and marketplace_plugin_name + # MUST be set for every marketplace-sourced dependency so the + # lockfile records supply-chain origin. Missing provenance + # would leave marketplace deps indistinguishable from direct + # Git refs, defeating audit and shadow-detection checks. if marketplace_provenance: for dep_key, prov in marketplace_provenance.items(): if dep_key in lockfile.dependencies: diff --git a/src/apm_cli/marketplace/resolver.py b/src/apm_cli/marketplace/resolver.py index 03b1cf12..2d05b46b 100644 --- a/src/apm_cli/marketplace/resolver.py +++ b/src/apm_cli/marketplace/resolver.py @@ -289,6 +289,26 @@ def resolve_marketplace_plugin( marketplace_name, ) + # ---- Version immutability check (advisory) ---- + from .version_pins import check_version_pin, record_version_pin + + previous_ref = check_version_pin( + marketplace_name, plugin_name, entry.version, entry.ref, + ) + if previous_ref is not None: + logger.warning( + "Version %s of %s@%s ref changed: was '%s', now '%s'. " + "This may indicate a ref swap attack.", + entry.version, + plugin_name, + marketplace_name, + previous_ref, + entry.ref, + ) + record_version_pin( + marketplace_name, plugin_name, entry.version, entry.ref, + ) + logger.debug( "Resolved %s@%s -> %s", plugin_name, @@ -296,4 +316,26 @@ def resolve_marketplace_plugin( canonical, ) + # -- Shadow detection (advisory) -- + # Warn when the same plugin name exists in other registered + # marketplaces. This helps users notice potential name-squatting + # where an attacker publishes a same-named plugin in a secondary + # marketplace. + try: + from .shadow_detector import detect_shadows + + shadows = detect_shadows( + plugin_name, marketplace_name, auth_resolver=auth_resolver + ) + for shadow in shadows: + logger.warning( + "Plugin '%s' also found in marketplace '%s'. " + "Verify you are installing from the intended source.", + plugin_name, + shadow.marketplace_name, + ) + except Exception: + # Shadow detection must never break installation + logger.debug("Shadow detection failed", exc_info=True) + return canonical, plugin diff --git a/src/apm_cli/marketplace/shadow_detector.py b/src/apm_cli/marketplace/shadow_detector.py new file mode 100644 index 00000000..8330fe4e --- /dev/null +++ b/src/apm_cli/marketplace/shadow_detector.py @@ -0,0 +1,75 @@ +"""Cross-marketplace shadow detection for plugin name squatting. + +When a user installs ``my-plugin@acme``, shadow detection checks whether +the same plugin name appears in *other* registered marketplaces. A match +is not necessarily malicious, but it warrants a warning so the user can +verify they are installing from the intended source. + +This module is advisory-only -- errors are logged at DEBUG level and +never propagate to the caller. +""" + +import logging +from dataclasses import dataclass +from typing import List, Optional + +from .client import fetch_or_cache +from .registry import get_registered_marketplaces + +logger = logging.getLogger(__name__) + + +@dataclass(frozen=True) +class ShadowMatch: + """A plugin name found in a secondary marketplace.""" + + marketplace_name: str + plugin_name: str + + +def detect_shadows( + plugin_name: str, + primary_marketplace: str, + *, + auth_resolver: Optional[object] = None, +) -> List[ShadowMatch]: + """Check registered marketplaces for duplicate plugin names. + + Iterates over every registered marketplace *except* + ``primary_marketplace`` and returns a :class:`ShadowMatch` for each + one that contains a plugin with the same name (case-insensitive). + + Uses :func:`fetch_or_cache` so cached manifests are reused and no + extra network round-trips are needed in the common case. + + Args: + plugin_name: The plugin name to search for. + primary_marketplace: Marketplace the user explicitly selected + (excluded from the scan). + auth_resolver: Optional ``AuthResolver`` forwarded to + :func:`fetch_or_cache`. + + Returns: + A list of :class:`ShadowMatch` instances (may be empty). + """ + shadows: List[ShadowMatch] = [] + for source in get_registered_marketplaces(): + if source.name.lower() == primary_marketplace.lower(): + continue + try: + manifest = fetch_or_cache(source, auth_resolver=auth_resolver) + match = manifest.find_plugin(plugin_name) + if match is not None: + shadows.append( + ShadowMatch( + marketplace_name=source.name, + plugin_name=match.name, + ) + ) + except Exception as exc: + logger.debug( + "Shadow check failed for marketplace '%s': %s", + source.name, + exc, + ) + return shadows diff --git a/src/apm_cli/marketplace/version_pins.py b/src/apm_cli/marketplace/version_pins.py new file mode 100644 index 00000000..5fcb4075 --- /dev/null +++ b/src/apm_cli/marketplace/version_pins.py @@ -0,0 +1,150 @@ +"""Version pin cache for marketplace plugin immutability checks. + +Records version-to-ref mappings per plugin per marketplace. When a +previously-seen version resolves to a *different* ref, a warning is +emitted -- this may indicate a ref-swap attack where an attacker +changed the git ref for an existing version entry. + +The pin file lives at ``~/.apm/cache/marketplace/version-pins.json`` +and has the structure:: + + { + "marketplace/plugin": { + "2.0.0": "abc123...", + "1.0.0": "789012..." + } + } + +All functions are **fail-open**: filesystem or JSON errors are logged +and never block resolution. +""" + +import json +import logging +import os +from typing import Optional + +logger = logging.getLogger(__name__) + +_PINS_FILENAME = "version-pins.json" + + +# ------------------------------------------------------------------ +# Path helpers +# ------------------------------------------------------------------ + + +def _pins_path(pins_dir: Optional[str] = None) -> str: + """Return the full path to the version-pins JSON file. + + Args: + pins_dir: Override directory for the pins file. When ``None``, + the default ``~/.apm/cache/marketplace/`` is used. + """ + if pins_dir is not None: + return os.path.join(pins_dir, _PINS_FILENAME) + + from ..config import CONFIG_DIR + + return os.path.join(CONFIG_DIR, "cache", "marketplace", _PINS_FILENAME) + + +def _pin_key(marketplace_name: str, plugin_name: str) -> str: + """Build the canonical dict key for a marketplace/plugin pair.""" + return f"{marketplace_name}/{plugin_name}".lower() + + +# ------------------------------------------------------------------ +# Load / save +# ------------------------------------------------------------------ + + +def load_version_pins(pins_dir: Optional[str] = None) -> dict: + """Load the version-pins file from disk. + + Returns an empty dict when the file is missing or contains invalid + JSON. Never raises. + """ + path = _pins_path(pins_dir) + if not os.path.exists(path): + return {} + try: + with open(path, "r") as fh: + data = json.load(fh) + if not isinstance(data, dict): + logger.debug("version-pins file is not a JSON object; ignoring") + return {} + return data + except (json.JSONDecodeError, OSError) as exc: + logger.debug("Failed to load version-pins: %s", exc) + return {} + + +def save_version_pins(pins: dict, pins_dir: Optional[str] = None) -> None: + """Persist *pins* to disk atomically. + + Writes to a temporary file first, then uses ``os.replace`` to move + it into place so readers never see a partial write. Errors are + logged and swallowed (advisory system). + """ + path = _pins_path(pins_dir) + tmp_path = path + ".tmp" + try: + os.makedirs(os.path.dirname(path), exist_ok=True) + with open(tmp_path, "w") as fh: + json.dump(pins, fh, indent=2) + os.replace(tmp_path, path) + except OSError as exc: + logger.debug("Failed to save version-pins: %s", exc) + + +# ------------------------------------------------------------------ +# Check / record +# ------------------------------------------------------------------ + + +def check_version_pin( + marketplace_name: str, + plugin_name: str, + version: str, + ref: str, + pins_dir: Optional[str] = None, +) -> Optional[str]: + """Check whether *ref* matches the previously-recorded pin. + + Returns: + The **previously pinned ref** if it differs from *ref* (possible + ref swap). ``None`` if this is the first time seeing the + version or the ref matches. + """ + pins = load_version_pins(pins_dir) + key = _pin_key(marketplace_name, plugin_name) + plugin_pins = pins.get(key) + if not isinstance(plugin_pins, dict): + return None + previous_ref = plugin_pins.get(version) + if previous_ref is None: + return None + if previous_ref == ref: + return None + return previous_ref + + +def record_version_pin( + marketplace_name: str, + plugin_name: str, + version: str, + ref: str, + pins_dir: Optional[str] = None, +) -> None: + """Store a version-to-ref mapping in the pin cache. + + Overwrites any existing pin for the same version (advisory system + -- we always record the current ref even if it changed). + """ + pins = load_version_pins(pins_dir) + key = _pin_key(marketplace_name, plugin_name) + if key not in pins or not isinstance(pins.get(key), dict): + pins[key] = {} + pins[key][version] = ref + save_version_pins(pins, pins_dir) diff --git a/tests/unit/marketplace/test_shadow_detector.py b/tests/unit/marketplace/test_shadow_detector.py new file mode 100644 index 00000000..89cf0425 --- /dev/null +++ b/tests/unit/marketplace/test_shadow_detector.py @@ -0,0 +1,312 @@ +"""Tests for cross-marketplace shadow detection. + +Covers: +- detect_shadows() with zero, one, and multiple shadow matches +- Case-insensitive plugin name matching +- Primary marketplace exclusion +- Graceful handling of fetch errors and empty registries +- Integration: resolver.py emits warnings on shadow detection +- Integration: install.py sets provenance fields on marketplace deps +""" + +import logging +from unittest.mock import MagicMock, patch + +import pytest + +from apm_cli.marketplace.models import ( + MarketplaceManifest, + MarketplacePlugin, + MarketplaceSource, +) +from apm_cli.marketplace.shadow_detector import ShadowMatch, detect_shadows + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_source(name, owner="org", repo="repo"): + return MarketplaceSource(name=name, owner=owner, repo=repo) + + +def _make_plugin(name): + return MarketplacePlugin(name=name, source="plugins/" + name) + + +def _make_manifest(name, plugins): + return MarketplaceManifest( + name=name, + plugins=tuple(_make_plugin(p) for p in plugins), + ) + + +# Maps marketplace source name -> MarketplaceManifest for fetch_or_cache mock +def _build_fetch_side_effect(manifests_by_name): + """Return a side_effect callable for fetch_or_cache.""" + + def _fetch(source, **kwargs): + return manifests_by_name[source.name] + + return _fetch + + +# --------------------------------------------------------------------------- +# detect_shadows -- unit tests +# --------------------------------------------------------------------------- + + +_PATCH_REGISTRY = "apm_cli.marketplace.shadow_detector.get_registered_marketplaces" +_PATCH_FETCH = "apm_cli.marketplace.shadow_detector.fetch_or_cache" + + +class TestDetectShadows: + """Unit tests for the detect_shadows function.""" + + def test_no_shadows(self): + """Plugin only in primary marketplace -- returns empty list.""" + sources = [_make_source("primary")] + manifests = {"primary": _make_manifest("primary", ["my-plugin"])} + + with ( + patch(_PATCH_REGISTRY, return_value=sources), + patch(_PATCH_FETCH, side_effect=_build_fetch_side_effect(manifests)), + ): + result = detect_shadows("my-plugin", "primary") + + assert result == [] + + def test_shadow_found(self): + """Plugin in primary + one other marketplace -- returns 1 match.""" + sources = [_make_source("primary"), _make_source("community")] + manifests = { + "primary": _make_manifest("primary", ["my-plugin"]), + "community": _make_manifest("community", ["my-plugin", "other"]), + } + + with ( + patch(_PATCH_REGISTRY, return_value=sources), + patch(_PATCH_FETCH, side_effect=_build_fetch_side_effect(manifests)), + ): + result = detect_shadows("my-plugin", "primary") + + assert len(result) == 1 + assert result[0] == ShadowMatch( + marketplace_name="community", plugin_name="my-plugin" + ) + + def test_multiple_shadows(self): + """Plugin in 3 marketplaces -- returns 2 matches (excludes primary).""" + sources = [ + _make_source("primary"), + _make_source("community"), + _make_source("third-party"), + ] + manifests = { + "primary": _make_manifest("primary", ["my-plugin"]), + "community": _make_manifest("community", ["my-plugin"]), + "third-party": _make_manifest("third-party", ["my-plugin"]), + } + + with ( + patch(_PATCH_REGISTRY, return_value=sources), + patch(_PATCH_FETCH, side_effect=_build_fetch_side_effect(manifests)), + ): + result = detect_shadows("my-plugin", "primary") + + assert len(result) == 2 + names = {s.marketplace_name for s in result} + assert names == {"community", "third-party"} + + def test_case_insensitive(self): + """Shadow detected even with different casing of marketplace name.""" + sources = [_make_source("Primary"), _make_source("community")] + manifests = { + "Primary": _make_manifest("Primary", ["My-Plugin"]), + "community": _make_manifest("community", ["my-plugin"]), + } + + with ( + patch(_PATCH_REGISTRY, return_value=sources), + patch(_PATCH_FETCH, side_effect=_build_fetch_side_effect(manifests)), + ): + # Primary name uses different casing -- should still be excluded + result = detect_shadows("my-plugin", "primary") + + assert len(result) == 1 + assert result[0].marketplace_name == "community" + + def test_primary_excluded(self): + """Primary marketplace never appears in results even if it matches.""" + sources = [_make_source("acme"), _make_source("other")] + manifests = { + "acme": _make_manifest("acme", ["sec-check"]), + "other": _make_manifest("other", []), + } + + with ( + patch(_PATCH_REGISTRY, return_value=sources), + patch(_PATCH_FETCH, side_effect=_build_fetch_side_effect(manifests)), + ): + result = detect_shadows("sec-check", "acme") + + assert result == [] + + def test_fetch_error_handled(self, caplog): + """One marketplace fails to fetch -- others still checked.""" + sources = [ + _make_source("primary"), + _make_source("broken"), + _make_source("good"), + ] + # Only "good" has a manifest; "broken" will raise + manifests = { + "good": _make_manifest("good", ["my-plugin"]), + } + + def _fetch(source, **kwargs): + if source.name == "broken": + raise ConnectionError("network down") + return manifests[source.name] + + with ( + patch(_PATCH_REGISTRY, return_value=sources), + patch(_PATCH_FETCH, side_effect=_fetch), + caplog.at_level(logging.DEBUG, logger="apm_cli.marketplace.shadow_detector"), + ): + result = detect_shadows("my-plugin", "primary") + + # "good" marketplace returned a match despite "broken" failing + assert len(result) == 1 + assert result[0].marketplace_name == "good" + # Verify the error was logged at DEBUG level + assert any("broken" in rec.message for rec in caplog.records) + + def test_no_registered_marketplaces(self): + """No marketplaces registered -- returns empty list.""" + with ( + patch(_PATCH_REGISTRY, return_value=[]), + patch(_PATCH_FETCH) as mock_fetch, + ): + result = detect_shadows("anything", "nonexistent") + + assert result == [] + mock_fetch.assert_not_called() + + def test_only_primary_registered(self): + """Only primary marketplace registered -- returns empty list.""" + sources = [_make_source("only-one")] + + with ( + patch(_PATCH_REGISTRY, return_value=sources), + patch(_PATCH_FETCH) as mock_fetch, + ): + result = detect_shadows("my-plugin", "only-one") + + assert result == [] + mock_fetch.assert_not_called() + + +# --------------------------------------------------------------------------- +# Integration: resolver.py shadow warning +# --------------------------------------------------------------------------- + + +class TestShadowDetectionInResolver: + """Verify resolver.py logs a warning when shadows are detected.""" + + def test_shadow_detection_in_resolver(self, caplog): + """resolve_marketplace_plugin emits a warning per shadow.""" + from apm_cli.marketplace.resolver import resolve_marketplace_plugin + + plugin = MarketplacePlugin( + name="sec-check", + source={"type": "github", "repo": "acme/sec-check", "ref": "main"}, + ) + manifest = MarketplaceManifest( + name="acme", plugins=(plugin,) + ) + source = MarketplaceSource(name="acme", owner="acme", repo="marketplace") + + shadow = ShadowMatch(marketplace_name="community", plugin_name="sec-check") + + with ( + patch( + "apm_cli.marketplace.resolver.get_marketplace_by_name", + return_value=source, + ), + patch( + "apm_cli.marketplace.resolver.fetch_or_cache", + return_value=manifest, + ), + patch( + "apm_cli.marketplace.shadow_detector.detect_shadows", + return_value=[shadow], + ) as mock_detect, + caplog.at_level(logging.WARNING, logger="apm_cli.marketplace.resolver"), + ): + canonical, resolved = resolve_marketplace_plugin( + "sec-check", "acme" + ) + + # Resolution succeeded + assert canonical == "acme/sec-check#main" + assert resolved.name == "sec-check" + + # Shadow detection was called with correct args + mock_detect.assert_called_once_with( + "sec-check", "acme", auth_resolver=None + ) + + # Warning emitted for the shadow + warnings = [r for r in caplog.records if r.levelno == logging.WARNING] + assert len(warnings) == 1 + assert "community" in warnings[0].message + assert "sec-check" in warnings[0].message + + +# --------------------------------------------------------------------------- +# Integration: install.py provenance fields +# --------------------------------------------------------------------------- + + +class TestProvenanceSetOnMarketplaceDeps: + """Verify install.py sets discovered_via and marketplace_plugin_name.""" + + def test_provenance_set_on_marketplace_deps(self): + """Marketplace provenance dict is correctly structured.""" + # This test validates the contract between the install command's + # marketplace interception and the lockfile provenance attachment. + # We verify the data shape, not the full install flow. + from apm_cli.deps.lockfile import LockedDependency + + # Simulate what install.py lines 169-173 produce + marketplace_name = "acme-tools" + plugin_name = "sec-check" + version_spec = "^2.0.0" + + marketplace_provenance = { + "discovered_via": marketplace_name, + "marketplace_plugin_name": plugin_name, + "version_spec": version_spec, + } + + # Simulate what install.py lines 2384-2390 do + dep = LockedDependency(repo_url="acme/sec-check") + dep.discovered_via = marketplace_provenance["discovered_via"] + dep.marketplace_plugin_name = marketplace_provenance[ + "marketplace_plugin_name" + ] + dep.version_spec = marketplace_provenance["version_spec"] + + # Security-critical: all provenance fields must be set + assert dep.discovered_via == "acme-tools" + assert dep.marketplace_plugin_name == "sec-check" + assert dep.version_spec == "^2.0.0" + + # Round-trip through serialization + d = dep.to_dict() + assert d["discovered_via"] == "acme-tools" + assert d["marketplace_plugin_name"] == "sec-check" + assert d["version_spec"] == "^2.0.0" diff --git a/tests/unit/marketplace/test_version_pins.py b/tests/unit/marketplace/test_version_pins.py new file mode 100644 index 00000000..c2cee282 --- /dev/null +++ b/tests/unit/marketplace/test_version_pins.py @@ -0,0 +1,371 @@ +"""Tests for version pin cache (immutability advisory). + +Covers: +- Loading from missing / corrupt / valid pin files +- Recording and persisting pins +- Detecting ref changes (possible ref swap) +- Multi-plugin isolation +- Atomic write via os.replace +- Integration with resolve_marketplace_plugin (warning logged on ref change) +""" + +import json +import logging +import os +from unittest.mock import MagicMock, patch + +import pytest + +from apm_cli.marketplace.version_pins import ( + _pin_key, + _pins_path, + check_version_pin, + load_version_pins, + record_version_pin, + save_version_pins, +) + + +# --------------------------------------------------------------------------- +# Unit tests -- load / save +# --------------------------------------------------------------------------- + + +class TestLoadVersionPins: + """Loading the pin file from disk.""" + + def test_load_empty_no_file(self, tmp_path): + """Missing file returns empty dict.""" + result = load_version_pins(pins_dir=str(tmp_path)) + assert result == {} + + def test_load_corrupt_json(self, tmp_path): + """Corrupt JSON returns empty dict without raising.""" + path = tmp_path / "version-pins.json" + path.write_text("{not valid json!!!") + result = load_version_pins(pins_dir=str(tmp_path)) + assert result == {} + + def test_load_non_dict_json(self, tmp_path): + """JSON that is not an object returns empty dict.""" + path = tmp_path / "version-pins.json" + path.write_text('["a list", "not a dict"]') + result = load_version_pins(pins_dir=str(tmp_path)) + assert result == {} + + def test_load_valid(self, tmp_path): + """Valid JSON is returned as-is.""" + data = {"mkt/plug": {"1.0.0": "abc123"}} + path = tmp_path / "version-pins.json" + path.write_text(json.dumps(data)) + result = load_version_pins(pins_dir=str(tmp_path)) + assert result == data + + +class TestSaveVersionPins: + """Saving the pin file to disk.""" + + def test_save_creates_file(self, tmp_path): + """Save creates the file if it does not exist.""" + pins = {"mkt/plug": {"1.0.0": "ref1"}} + save_version_pins(pins, pins_dir=str(tmp_path)) + + path = tmp_path / "version-pins.json" + assert path.exists() + assert json.loads(path.read_text()) == pins + + def test_save_creates_parent_dirs(self, tmp_path): + """Save creates intermediate directories if needed.""" + nested = tmp_path / "a" / "b" + pins = {"mkt/plug": {"2.0.0": "ref2"}} + save_version_pins(pins, pins_dir=str(nested)) + + path = nested / "version-pins.json" + assert path.exists() + assert json.loads(path.read_text()) == pins + + +# --------------------------------------------------------------------------- +# Unit tests -- record / check +# --------------------------------------------------------------------------- + + +class TestRecordAndCheck: + """Recording pins and checking for ref changes.""" + + def test_record_and_load(self, tmp_path): + """Record a pin and verify it persists on disk.""" + record_version_pin("mkt", "plug", "1.0.0", "sha-aaa", pins_dir=str(tmp_path)) + pins = load_version_pins(pins_dir=str(tmp_path)) + assert pins["mkt/plug"]["1.0.0"] == "sha-aaa" + + def test_check_new_pin(self, tmp_path): + """First time seeing a version returns None (no warning).""" + result = check_version_pin("mkt", "plug", "1.0.0", "sha-aaa", pins_dir=str(tmp_path)) + assert result is None + + def test_check_matching_pin(self, tmp_path): + """Same ref as previously recorded returns None.""" + record_version_pin("mkt", "plug", "1.0.0", "sha-aaa", pins_dir=str(tmp_path)) + result = check_version_pin("mkt", "plug", "1.0.0", "sha-aaa", pins_dir=str(tmp_path)) + assert result is None + + def test_check_changed_pin(self, tmp_path): + """Different ref returns the previous (old) ref string.""" + record_version_pin("mkt", "plug", "1.0.0", "sha-aaa", pins_dir=str(tmp_path)) + result = check_version_pin("mkt", "plug", "1.0.0", "sha-bbb", pins_dir=str(tmp_path)) + assert result == "sha-aaa" + + def test_record_overwrites(self, tmp_path): + """Recording the same version twice overwrites the old ref.""" + record_version_pin("mkt", "plug", "1.0.0", "sha-aaa", pins_dir=str(tmp_path)) + record_version_pin("mkt", "plug", "1.0.0", "sha-bbb", pins_dir=str(tmp_path)) + pins = load_version_pins(pins_dir=str(tmp_path)) + assert pins["mkt/plug"]["1.0.0"] == "sha-bbb" + + def test_multiple_plugins(self, tmp_path): + """Different plugins do not interfere with each other.""" + record_version_pin("mkt", "alpha", "1.0.0", "ref-a", pins_dir=str(tmp_path)) + record_version_pin("mkt", "beta", "1.0.0", "ref-b", pins_dir=str(tmp_path)) + + assert check_version_pin("mkt", "alpha", "1.0.0", "ref-a", pins_dir=str(tmp_path)) is None + assert check_version_pin("mkt", "beta", "1.0.0", "ref-b", pins_dir=str(tmp_path)) is None + # Alpha ref changed, beta unchanged + assert check_version_pin("mkt", "alpha", "1.0.0", "ref-x", pins_dir=str(tmp_path)) == "ref-a" + assert check_version_pin("mkt", "beta", "1.0.0", "ref-b", pins_dir=str(tmp_path)) is None + + def test_multiple_versions_same_plugin(self, tmp_path): + """Different versions of the same plugin are tracked independently.""" + record_version_pin("mkt", "plug", "1.0.0", "ref-v1", pins_dir=str(tmp_path)) + record_version_pin("mkt", "plug", "2.0.0", "ref-v2", pins_dir=str(tmp_path)) + + assert check_version_pin("mkt", "plug", "1.0.0", "ref-v1", pins_dir=str(tmp_path)) is None + assert check_version_pin("mkt", "plug", "2.0.0", "ref-v2", pins_dir=str(tmp_path)) is None + # Only v1 ref changed + assert check_version_pin("mkt", "plug", "1.0.0", "ref-new", pins_dir=str(tmp_path)) == "ref-v1" + assert check_version_pin("mkt", "plug", "2.0.0", "ref-v2", pins_dir=str(tmp_path)) is None + + +# --------------------------------------------------------------------------- +# Unit tests -- key normalization +# --------------------------------------------------------------------------- + + +class TestPinKey: + """Pin key construction and normalization.""" + + def test_lowercase(self): + assert _pin_key("MKT", "Plugin") == "mkt/plugin" + + def test_already_lower(self): + assert _pin_key("mkt", "plugin") == "mkt/plugin" + + +# --------------------------------------------------------------------------- +# Unit tests -- pins_path +# --------------------------------------------------------------------------- + + +class TestPinsPath: + """Path construction for the pins file.""" + + def test_custom_dir(self, tmp_path): + result = _pins_path(pins_dir=str(tmp_path)) + assert result == os.path.join(str(tmp_path), "version-pins.json") + + def test_default_dir(self): + """Default path (no pins_dir) includes version-pins.json under CONFIG_DIR.""" + with patch("apm_cli.config.CONFIG_DIR", "/fake/.apm"): + result = _pins_path(pins_dir=None) + assert result == os.path.join("/fake/.apm", "cache", "marketplace", "version-pins.json") + + +# --------------------------------------------------------------------------- +# Atomic write +# --------------------------------------------------------------------------- + + +class TestAtomicWrite: + """Verify save uses atomic write pattern (tmp + os.replace).""" + + def test_atomic_write_uses_replace(self, tmp_path): + """os.replace is called to atomically move the temp file.""" + pins = {"mkt/plug": {"1.0.0": "ref1"}} + + with patch("apm_cli.marketplace.version_pins.os.replace", wraps=os.replace) as mock_replace: + save_version_pins(pins, pins_dir=str(tmp_path)) + mock_replace.assert_called_once() + args = mock_replace.call_args[0] + assert args[0].endswith(".tmp") + assert args[1].endswith("version-pins.json") + + def test_no_tmp_file_remains(self, tmp_path): + """After save, no .tmp file should remain on disk.""" + save_version_pins({"k": {"v": "r"}}, pins_dir=str(tmp_path)) + remaining = list(tmp_path.iterdir()) + assert all(not f.name.endswith(".tmp") for f in remaining) + + +# --------------------------------------------------------------------------- +# Fail-open behavior +# --------------------------------------------------------------------------- + + +class TestFailOpen: + """Advisory system must never raise on I/O errors.""" + + def test_save_to_readonly_dir_does_not_raise(self, tmp_path): + """Save to an unwritable location logs and returns without error.""" + # Use a path that does not exist and cannot be created + bad_dir = "/dev/null/impossible" + # Should not raise + save_version_pins({"k": {"v": "r"}}, pins_dir=bad_dir) + + def test_check_with_corrupt_file_returns_none(self, tmp_path): + """check_version_pin with corrupt file returns None (no warning).""" + path = tmp_path / "version-pins.json" + path.write_text("CORRUPT!!!") + result = check_version_pin("mkt", "plug", "1.0.0", "ref", pins_dir=str(tmp_path)) + assert result is None + + def test_check_with_non_dict_plugin_entry(self, tmp_path): + """If the plugin entry is not a dict, return None gracefully.""" + data = {"mkt/plug": "not-a-dict"} + path = tmp_path / "version-pins.json" + path.write_text(json.dumps(data)) + result = check_version_pin("mkt", "plug", "1.0.0", "ref", pins_dir=str(tmp_path)) + assert result is None + + +# --------------------------------------------------------------------------- +# Integration -- resolver emits warning on ref change +# --------------------------------------------------------------------------- + + +class TestResolverIntegration: + """Verify resolve_marketplace_plugin logs a warning when a version ref changes.""" + + def _make_source(self): + from apm_cli.marketplace.models import MarketplaceSource + + return MarketplaceSource(name="test-mkt", owner="acme-org", repo="marketplace") + + def _make_manifest(self, plugin): + from apm_cli.marketplace.models import MarketplaceManifest + + return MarketplaceManifest( + name="test-mkt", + plugins=(plugin,), + plugin_root="", + ) + + def _make_plugin(self, ref="sha-original"): + from apm_cli.marketplace.models import MarketplacePlugin, VersionEntry + + return MarketplacePlugin( + name="my-plugin", + source={"type": "github", "repo": "acme-org/my-plugin", "ref": "main"}, + versions=(VersionEntry(version="2.0.0", ref=ref),), + source_marketplace="test-mkt", + ) + + def test_no_warning_on_first_install(self, tmp_path, caplog): + """First install of a version should not log a warning.""" + from apm_cli.marketplace.resolver import resolve_marketplace_plugin + + plugin = self._make_plugin(ref="sha-original") + source = self._make_source() + manifest = self._make_manifest(plugin) + + with ( + patch( + "apm_cli.marketplace.resolver.get_marketplace_by_name", + return_value=source, + ), + patch( + "apm_cli.marketplace.resolver.fetch_or_cache", + return_value=manifest, + ), + patch( + "apm_cli.marketplace.version_pins.load_version_pins", + return_value={}, + ), + patch( + "apm_cli.marketplace.version_pins.save_version_pins", + ), + ): + with caplog.at_level(logging.WARNING, logger="apm_cli.marketplace.resolver"): + resolve_marketplace_plugin( + "my-plugin", "test-mkt", version_spec="2.0.0" + ) + assert "ref changed" not in caplog.text + + def test_warning_on_ref_change(self, tmp_path, caplog): + """When a known version ref changes, a warning is logged.""" + from apm_cli.marketplace.resolver import resolve_marketplace_plugin + + plugin = self._make_plugin(ref="sha-evil") + source = self._make_source() + manifest = self._make_manifest(plugin) + + # Simulate a previously recorded pin with a different ref + existing_pins = {"test-mkt/my-plugin": {"2.0.0": "sha-original"}} + + with ( + patch( + "apm_cli.marketplace.resolver.get_marketplace_by_name", + return_value=source, + ), + patch( + "apm_cli.marketplace.resolver.fetch_or_cache", + return_value=manifest, + ), + patch( + "apm_cli.marketplace.version_pins.load_version_pins", + return_value=existing_pins, + ), + patch( + "apm_cli.marketplace.version_pins.save_version_pins", + ), + ): + with caplog.at_level(logging.WARNING, logger="apm_cli.marketplace.resolver"): + resolve_marketplace_plugin( + "my-plugin", "test-mkt", version_spec="2.0.0" + ) + assert "ref changed" in caplog.text + assert "sha-original" in caplog.text + assert "sha-evil" in caplog.text + assert "ref swap attack" in caplog.text + + def test_no_warning_when_ref_matches(self, tmp_path, caplog): + """Same ref as previously pinned produces no warning.""" + from apm_cli.marketplace.resolver import resolve_marketplace_plugin + + plugin = self._make_plugin(ref="sha-original") + source = self._make_source() + manifest = self._make_manifest(plugin) + + existing_pins = {"test-mkt/my-plugin": {"2.0.0": "sha-original"}} + + with ( + patch( + "apm_cli.marketplace.resolver.get_marketplace_by_name", + return_value=source, + ), + patch( + "apm_cli.marketplace.resolver.fetch_or_cache", + return_value=manifest, + ), + patch( + "apm_cli.marketplace.version_pins.load_version_pins", + return_value=existing_pins, + ), + patch( + "apm_cli.marketplace.version_pins.save_version_pins", + ), + ): + with caplog.at_level(logging.WARNING, logger="apm_cli.marketplace.resolver"): + resolve_marketplace_plugin( + "my-plugin", "test-mkt", version_spec="2.0.0" + ) + assert "ref changed" not in caplog.text From 8fd999464af9944eeb481e38d0721141c7782501 Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Sat, 11 Apr 2026 17:42:20 +0200 Subject: [PATCH 4/8] docs: marketplace versioning guides, CLI reference, and changelog Update marketplaces guide with versioned plugins schema, semver range install syntax, version viewing, publish/validate commands, and security hardening sections (immutability + shadow detection). Update CLI reference with marketplace publish and validate subcommands, version specifier syntax in install/view/outdated commands. Add version_spec field to lockfile spec. Update apm-guide skill resources with new marketplace commands and version specifier table. Add changelog entries for all Phase 1-3 features under [Unreleased]. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 8 ++ docs/src/content/docs/guides/marketplaces.md | 114 +++++++++++++++++- .../content/docs/reference/cli-commands.md | 70 ++++++++++- .../content/docs/reference/lockfile-spec.md | 1 + .../.apm/skills/apm-usage/commands.md | 4 + .../.apm/skills/apm-usage/dependencies.md | 16 +++ 6 files changed, 209 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ebf1a08..9b30295d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - `apm install` now automatically discovers and deploys local `.apm/` primitives (skills, instructions, agents, prompts, hooks, commands) to target directories, with local content taking priority over dependencies on collision (#626, #644) +- Marketplace-based version management: plugins can declare `versions[]` arrays with semver version-to-ref mappings (#514) +- Semver range resolution for marketplace installs: `apm install plugin@marketplace#^2.0.0` supports `^`, `~`, `>=`, `>`, `<`, `<=`, `!=`, exact, and compound ranges (#514) +- `apm view plugin@marketplace` displays available marketplace versions with their refs (#514) +- `apm outdated` checks marketplace versions and shows a "Source" column distinguishing marketplace vs git updates (#514) +- `apm marketplace publish` command to add version entries to `marketplace.json` from `apm.yml` defaults and git HEAD (#514) +- `apm marketplace validate` command with schema validation, semver format checks, and duplicate detection (#514) +- Version immutability advisory: caches version-to-ref pins and warns when a previously pinned version's ref changes (#514) +- Multi-marketplace shadow detection: warns when the same plugin name appears in multiple registered marketplaces (#514) ### Fixed diff --git a/docs/src/content/docs/guides/marketplaces.md b/docs/src/content/docs/guides/marketplaces.md index a2607822..1b0a9a3a 100644 --- a/docs/src/content/docs/guides/marketplaces.md +++ b/docs/src/content/docs/guides/marketplaces.md @@ -63,6 +63,27 @@ Marketplaces can declare a `metadata.pluginRoot` field to specify the base direc With `pluginRoot` set to `./plugins`, the source `"my-tool"` resolves to `owner/repo/plugins/my-tool`. Sources that already contain a path separator (e.g. `./custom/path`) are not affected by `pluginRoot`. +### Versioned plugins + +Plugins can declare a `versions` array that maps semver versions to Git refs: + +```json +{ + "name": "code-review", + "description": "Automated code review agent", + "source": { "type": "github", "repo": "acme/code-review-plugin" }, + "versions": [ + { "version": "2.1.0", "ref": "abc123def456" }, + { "version": "2.0.0", "ref": "v2.0.0" }, + { "version": "1.0.0", "ref": "v1.0.0" } + ] +} +``` + +When `versions` is present, APM uses semver resolution instead of the source-level ref. The `ref` field accepts commit SHAs, tags, or branch names. List versions newest-first by convention. + +Plugins without `versions` continue using the source-level ref -- this is fully backward compatible. + ## Register a marketplace ```bash @@ -125,13 +146,38 @@ use `apm marketplace browse ` instead. Use the `NAME@MARKETPLACE` syntax to install a plugin from a specific marketplace: ```bash +# Install latest version apm install code-review@acme-plugins + +# Install exact version +apm install code-review@acme-plugins#2.0.0 + +# Install compatible range (^2.0.0 means >=2.0.0, <3.0.0) +apm install code-review@acme-plugins#^2.0.0 + +# Install with tilde range (~2.1.0 means >=2.1.0, <2.2.0) +apm install code-review@acme-plugins#~2.1.0 + +# Compound constraint +apm install code-review@acme-plugins#>=1.0.0,<3.0.0 ``` -APM resolves the plugin name against the marketplace index, fetches the underlying Git repository, and installs it as a standard APM dependency. The resolved source appears in `apm.yml` and `apm.lock.yaml` just like any direct dependency. +The `#` separator carries a version specifier when the plugin declares `versions`, or a raw git ref when it does not. Plugins without `versions` continue to work as before. + +APM resolves the plugin name against the marketplace index, fetches the underlying Git repository at the resolved ref, and installs it as a standard APM dependency. The resolved source appears in `apm.yml` and `apm.lock.yaml` just like any direct dependency. For full `apm install` options, see [CLI Commands](../../reference/cli-commands/). +## View versions + +Show available versions for a marketplace plugin: + +```bash +apm view code-review@acme-plugins +``` + +Displays a table of versions with their refs, sorted newest-first. For plugins without `versions`, shows remote tags and branches. + ## Provenance tracking Marketplace-resolved plugins are tracked in `apm.lock.yaml` with full provenance: @@ -187,3 +233,69 @@ apm marketplace remove acme-plugins --yes ``` Removing a marketplace does not uninstall plugins previously installed from it. Those plugins remain pinned in `apm.lock.yaml` to their resolved Git sources. + +## Publish versions + +Add a version entry to a marketplace's `marketplace.json`. Reads defaults from `apm.yml` and resolves the current Git HEAD: + +```bash +# Publish current version (reads apm.yml + git HEAD) +apm marketplace publish --marketplace acme-plugins + +# Publish with explicit values +apm marketplace publish --marketplace acme-plugins --plugin code-review --version 2.1.0 --ref abc123 + +# Preview without writing +apm marketplace publish --marketplace acme-plugins --dry-run +``` + +The command appends a version entry to the plugin's `versions` array. Use `--force` to overwrite an existing version entry with a different ref. + +For full option details, see [CLI Commands](../../reference/cli-commands/). + +## Validate a marketplace + +Check a marketplace manifest for schema errors, invalid semver formats, and duplicate entries: + +```bash +apm marketplace validate acme-plugins + +# Verbose output +apm marketplace validate acme-plugins --verbose +``` + +Catches: missing required fields, malformed version strings, duplicate versions within a plugin, and duplicate plugin names (case-insensitive). + +:::note[Planned] +The `--check-refs` flag will verify that version refs are reachable over the network. It is accepted but not yet implemented. +::: + +For full option details, see [CLI Commands](../../reference/cli-commands/). + +## Security + +### Version immutability + +APM caches version-to-ref mappings in `~/.apm/cache/marketplace/version-pins.json`. On subsequent installs, APM compares the marketplace ref against the cached pin. If a version's ref has changed, APM warns: + +``` +WARNING: Version 2.0.0 of code-review@acme-plugins ref changed: was 'v2.0.0', now 'deadbeef'. This may indicate a ref swap attack. +``` + +This detects marketplace maintainers (or compromised accounts) silently pointing an existing version at different code. + +### Shadow detection + +When installing a marketplace plugin, APM checks all other registered marketplaces for plugins with the same name. A match produces a warning: + +``` +WARNING: Plugin 'code-review' also found in marketplace 'other-plugins'. Verify you are installing from the intended source. +``` + +Shadow detection runs automatically during install -- no configuration required. + +### Best practices + +- **Use commit SHAs as refs** -- tags and branches can be moved; commit SHAs cannot. +- **Keep plugin names unique across marketplaces** -- avoids shadow warnings and reduces confusion. +- **Review immutability warnings** -- a changed ref for an existing version is a strong signal of tampering. diff --git a/docs/src/content/docs/reference/cli-commands.md b/docs/src/content/docs/reference/cli-commands.md index 438ded4f..a279e47c 100644 --- a/docs/src/content/docs/reference/cli-commands.md +++ b/docs/src/content/docs/reference/cli-commands.md @@ -81,7 +81,7 @@ apm install [PACKAGES...] [OPTIONS] ``` **Arguments:** -- `PACKAGES` - Optional APM packages to add and install. Accepts shorthand (`owner/repo`), HTTPS URLs, SSH URLs, FQDN shorthand (`host/owner/repo`), local filesystem paths (`./path`, `../path`, `/absolute/path`, `~/path`), or marketplace references (`NAME@MARKETPLACE`). All forms are normalized to canonical format in `apm.yml`. +- `PACKAGES` - Optional APM packages to add and install. Accepts shorthand (`owner/repo`), HTTPS URLs, SSH URLs, FQDN shorthand (`host/owner/repo`), local filesystem paths (`./path`, `../path`, `/absolute/path`, `~/path`), or marketplace references (`NAME@MARKETPLACE[#version_spec]`). All forms are normalized to canonical format in `apm.yml`. **Options:** - `--runtime TEXT` - Target specific runtime only (copilot, codex, vscode) @@ -167,6 +167,9 @@ apm install -g microsoft/apm-sample-package # Install a plugin from a registered marketplace apm install code-review@acme-plugins + +# Install a specific version from a marketplace +apm install code-review@acme-plugins#^2.0.0 ``` **Auto-Bootstrap Behavior:** @@ -621,7 +624,7 @@ apm view PACKAGE [FIELD] [OPTIONS] ``` **Arguments:** -- `PACKAGE` - Package name, usually `owner/repo` or a short repo name +- `PACKAGE` - Package name: `owner/repo`, short repo name, or `NAME@MARKETPLACE` for marketplace plugins - `FIELD` - Optional field selector. Supported value: `versions` **Options:** @@ -638,6 +641,9 @@ apm view apm-sample-package # List remote tags and branches without cloning apm view microsoft/apm-sample-package versions +# View available versions for a marketplace plugin +apm view code-review@acme-plugins + # Inspect a package from user scope apm view microsoft/apm-sample-package -g ``` @@ -647,6 +653,7 @@ apm view microsoft/apm-sample-package -g - Shows package name, version, description, source, install path, context files, workflows, and hooks - `versions` lists remote tags and branches without cloning the repository - `versions` does not require the package to be installed locally +- `NAME@MARKETPLACE` syntax shows the plugin's declared `versions` array sorted newest-first; for plugins without `versions`, falls back to remote tags and branches ### `apm outdated` - Check locked dependencies for updates @@ -680,8 +687,10 @@ apm outdated -j 8 - Reads the current lockfile (`apm.lock.yaml`; legacy `apm.lock` is migrated automatically) - For tag-pinned deps: compares the locked semver tag against the latest available remote tag - For branch-pinned deps: compares the locked commit SHA against the remote branch tip SHA +- For marketplace deps with `versions`: compares against the latest version in the marketplace (respects `version_spec` range when set) - For deps with no ref: compares against the default branch (main/master) tip SHA -- Displays `Package`, `Current`, `Latest`, and `Status` columns +- Displays `Package`, `Current`, `Latest`, `Status`, and `Source` columns +- `Source` shows `marketplace: ` for marketplace-sourced deps - Status values are `up-to-date`, `outdated`, and `unknown` - Local dependencies and Artifactory dependencies are skipped @@ -1060,6 +1069,61 @@ apm marketplace remove acme-plugins apm marketplace remove acme-plugins --yes ``` +#### `apm marketplace publish` - Publish a version entry + +Add a version entry to a plugin's `versions` array in `marketplace.json`. Reads defaults from `apm.yml` and resolves the current Git HEAD. + +```bash +apm marketplace publish [OPTIONS] +``` + +**Options:** +- `-m, --marketplace TEXT` - Target marketplace name +- `-p, --plugin TEXT` - Plugin name in the marketplace (default: `name` from `apm.yml`) +- `--version TEXT` - Version to publish as semver `X.Y.Z` (default: `version` from `apm.yml`) +- `--ref TEXT` - Git ref or commit SHA (default: current HEAD) +- `--force` - Overwrite existing version entry with a different ref +- `--dry-run` - Show what would be published without making changes +- `-v, --verbose` - Show detailed output + +**Examples:** +```bash +# Publish current version (reads apm.yml + git HEAD) +apm marketplace publish --marketplace acme-plugins + +# Publish with explicit values +apm marketplace publish -m acme-plugins --plugin code-review --version 2.1.0 --ref abc123 + +# Preview without writing +apm marketplace publish -m acme-plugins --dry-run +``` + +See the [Marketplaces guide](../../guides/marketplaces/) for version schema details. + +#### `apm marketplace validate` - Validate a marketplace manifest + +Validate `marketplace.json` for schema errors, invalid semver formats, duplicate versions, and duplicate plugin names. + +```bash +apm marketplace validate NAME [OPTIONS] +``` + +**Arguments:** +- `NAME` - Name of the marketplace to validate + +**Options:** +- `--check-refs` - Verify version refs are reachable (network). *Not yet implemented.* +- `-v, --verbose` - Show detailed output + +**Examples:** +```bash +# Validate a marketplace +apm marketplace validate acme-plugins + +# Verbose output +apm marketplace validate acme-plugins --verbose +``` + ### `apm search` - Search plugins in a marketplace Search for plugins by name or description within a specific marketplace. diff --git a/docs/src/content/docs/reference/lockfile-spec.md b/docs/src/content/docs/reference/lockfile-spec.md index bd1a228d..3ea274af 100644 --- a/docs/src/content/docs/reference/lockfile-spec.md +++ b/docs/src/content/docs/reference/lockfile-spec.md @@ -125,6 +125,7 @@ fields: | `is_dev` | boolean | MAY | `true` if the dependency was resolved through [`devDependencies`](../manifest-schema/#5-devdependencies). Omitted when `false`. Dev deps are excluded from `apm pack --format plugin` bundles. | | `deployed_files` | array of strings | MUST | Every file path APM deployed for this dependency, relative to project root. | | `source` | string | MAY | Dependency source. `"local"` for local path dependencies. Omitted for remote (git) dependencies. | +| `version_spec` | string | MAY | Original semver range from the install specifier (e.g., `"^2.0.0"`). Present only for marketplace dependencies installed with a version constraint. Used by `apm outdated` to evaluate updates within the pinned range. | | `local_path` | string | MAY | Filesystem path (relative or absolute) to the local package. Present only when `source` is `"local"`. | Fields with empty or default values (empty strings, `false` booleans, empty diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index 029891bb..4820e204 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -57,7 +57,11 @@ | `apm marketplace browse NAME` | Browse marketplace packages | -- | | `apm marketplace update [NAME]` | Update marketplace index | -- | | `apm marketplace remove NAME` | Remove a marketplace | `-y` skip confirm | +| `apm marketplace publish` | Publish version to marketplace.json | `-m MARKETPLACE`, `--version`, `--ref`, `--force`, `--dry-run` | +| `apm marketplace validate NAME` | Validate marketplace manifest | `--check-refs`, `-v` | | `apm search QUERY@MARKETPLACE` | Search marketplace | `--limit N` | +| `apm install NAME@MKT#^X.Y.Z` | Install with semver range | Supports `^`, `~`, `>=`, exact | +| `apm view NAME@MARKETPLACE` | View marketplace versions | -- | ## MCP servers diff --git a/packages/apm-guide/.apm/skills/apm-usage/dependencies.md b/packages/apm-guide/.apm/skills/apm-usage/dependencies.md index 99abf2a8..8f6a0813 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/dependencies.md +++ b/packages/apm-guide/.apm/skills/apm-usage/dependencies.md @@ -115,6 +115,22 @@ dependencies: | Branch | `owner/repo#main` | Development -- tracks latest | | Commit SHA | `owner/repo#abc123d` | Maximum reproducibility | | No ref | `owner/repo` | Resolves default branch at install time | +| Marketplace semver | `plugin@marketplace#^2.0.0` | Marketplace plugins with `versions[]` | + +## Marketplace version specifiers + +When a marketplace plugin declares `versions[]`, the `#` suffix is a semver range: + +| Specifier | Meaning | Example | +|-----------|---------|---------| +| `2.0.0` | Exact version | `plugin@mkt#2.0.0` | +| `^2.0.0` | Compatible (`>=2.0.0, <3.0.0`) | `plugin@mkt#^2.0.0` | +| `~2.1.0` | Patch-level (`>=2.1.0, <2.2.0`) | `plugin@mkt#~2.1.0` | +| `>=1.5.0` | Minimum version | `plugin@mkt#>=1.5.0` | +| `>=1.0.0,<3.0.0` | Compound range | `plugin@mkt#>=1.0.0,<3.0.0` | +| *(omitted)* | Latest version | `plugin@mkt` | + +Plugins without `versions[]` continue using the source-level ref (backward compatible). ## What the lockfile pins From b21b34fd8f53c536c9d196a25449dc9f328bd012 Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Sat, 11 Apr 2026 18:27:57 +0200 Subject: [PATCH 5/8] fix: marketplace auth and outdated version detection - Use auth-first for marketplace fetch (private org repos return 404 unauthenticated, not 403, so unauth_first swallowed the error) - Fall back to version_spec when version field is absent in lockfile for marketplace outdated checks (strip range prefixes like ^, ~) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/commands/outdated.py | 4 +++- src/apm_cli/marketplace/client.py | 7 ++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/apm_cli/commands/outdated.py b/src/apm_cli/commands/outdated.py index bd7f8089..75dba8f2 100644 --- a/src/apm_cli/commands/outdated.py +++ b/src/apm_cli/commands/outdated.py @@ -66,9 +66,11 @@ def _check_marketplace_versions(dep, verbose): if not dep.discovered_via or not dep.marketplace_plugin_name: return None - current_ver = dep.version or "" + current_ver = dep.version or dep.version_spec or "" if not current_ver: return None + # Strip range prefixes (^, ~, >=) when using version_spec as current + current_ver = current_ver.lstrip("^~>= Date: Sat, 11 Apr 2026 19:30:33 +0200 Subject: [PATCH 6/8] fix: resolve 7 bugs from marketplace versioning integration testing B1: Add resolved_version field to LockedDependency and lockfile YAML. Populated during marketplace version resolution (3-tuple return from resolve_marketplace_plugin). Enables lockfile-based version verification. B2: Exit code 1 when all packages fail validation (was silently returning 0). B3: Show 'Resolved version: X.Y.Z' in verbose install output for marketplace packages alongside the resolved ref. B4: Route 'apm view plugin@marketplace' (no subfield) to marketplace version display instead of failing with 'not found in apm_modules'. B5: Show 'best in range' upgrade in outdated output when latest version is outside the pinned range (e.g., '2.0.0 (outside ^1.0.0; best in range: 1.2.0)'). B6: Use resolved_version as primary fallback in outdated, with robust regex extraction from version_spec for compound ranges. B7: Route security warnings (immutability, shadow detection) through CLI CommandLogger via warning_handler callback, visible at -v level. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/commands/install.py | 15 +- src/apm_cli/commands/outdated.py | 43 +++- src/apm_cli/commands/view.py | 9 + src/apm_cli/deps/lockfile.py | 4 + src/apm_cli/marketplace/resolver.py | 52 ++-- .../test_marketplace_install_integration.py | 128 ++++++++++ .../unit/marketplace/test_shadow_detector.py | 2 +- .../marketplace/test_versioned_resolver.py | 241 +++++++++++++++++- tests/unit/test_outdated_marketplace.py | 192 ++++++++++++++ tests/unit/test_view_command.py | 126 +++++++++ 10 files changed, 777 insertions(+), 35 deletions(-) diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index de20c5c2..f3947c3e 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -156,16 +156,20 @@ def _validate_and_add_packages_to_apm_yml(packages, dry_run=False, dev=False, lo logger.verbose_detail( f" Resolving {plugin_name}@{marketplace_name} via marketplace..." ) - canonical_str, resolved_plugin = resolve_marketplace_plugin( + canonical_str, resolved_plugin, resolved_version = resolve_marketplace_plugin( plugin_name, marketplace_name, version_spec=version_spec, auth_resolver=auth_resolver, + warning_handler=logger.warning if logger else None, ) if logger: logger.verbose_detail( f" Resolved to: {canonical_str}" ) + # Show resolved version when available (marketplace installs) + # resolved_version is added to provenance by Bug B1; use + # safe .get() so this works before and after B1 lands. # Security-critical: record marketplace provenance so # the lockfile tracks where each dependency was # discovered. These fields enable supply-chain audits @@ -174,7 +178,13 @@ def _validate_and_add_packages_to_apm_yml(packages, dry_run=False, dev=False, lo "discovered_via": marketplace_name, "marketplace_plugin_name": plugin_name, "version_spec": version_spec, + "resolved_version": resolved_version, } + resolved_ver = marketplace_provenance.get("resolved_version") + if resolved_ver and logger: + logger.verbose_detail( + f" Resolved version: {resolved_ver}" + ) package = canonical_str except Exception as mkt_err: reason = str(mkt_err) @@ -712,7 +722,7 @@ def install(ctx, packages, runtime, exclude, only, update, dry_run, force, verbo ) # Short-circuit: all packages failed validation — nothing to install if outcome.all_failed: - return + sys.exit(1) # Note: Empty validated_packages is OK if packages are already in apm.yml # We'll proceed with installation from apm.yml to ensure everything is synced @@ -2633,6 +2643,7 @@ def _collect_descendants(node, visited=None): lockfile.dependencies[dep_key].discovered_via = prov.get("discovered_via") lockfile.dependencies[dep_key].marketplace_plugin_name = prov.get("marketplace_plugin_name") lockfile.dependencies[dep_key].version_spec = prov.get("version_spec") + lockfile.dependencies[dep_key].resolved_version = prov.get("resolved_version") # Selectively merge entries from the existing lockfile: # - For partial installs (only_packages): preserve all old entries # (sequential install — only the specified package was processed). diff --git a/src/apm_cli/commands/outdated.py b/src/apm_cli/commands/outdated.py index 75dba8f2..f31cca1a 100644 --- a/src/apm_cli/commands/outdated.py +++ b/src/apm_cli/commands/outdated.py @@ -66,11 +66,14 @@ def _check_marketplace_versions(dep, verbose): if not dep.discovered_via or not dep.marketplace_plugin_name: return None - current_ver = dep.version or dep.version_spec or "" + current_ver = getattr(dep, "resolved_version", None) or dep.version or "" + if not current_ver and dep.version_spec: + # Extract base version from spec - take first version-like number + match = re.search(r"(\d+\.\d+\.\d+)", dep.version_spec) + if match: + current_ver = match.group(1) if not current_ver: return None - # Strip range prefixes (^, ~, >=) when using version_spec as current - current_ver = current_ver.lstrip("^~>= current_parsed and _version_matches( + parsed, constraints, + ): + if ( + best_in_range_parsed is None + or parsed > best_in_range_parsed + ): + best_in_range_parsed = parsed + best_in_range = entry + + if ( + best_in_range is not None + and best_in_range_parsed is not None + and best_in_range_parsed > current_parsed + ): + latest_display = ( + f"{best_entry.version} (outside range {version_spec}; " + f"best in range: {best_in_range.version})" + ) + else: + latest_display = ( + f"{best_entry.version} (outside range {version_spec})" + ) extra = [e.version for e in plugin.versions[:10]] if verbose else [] return ( diff --git a/src/apm_cli/commands/view.py b/src/apm_cli/commands/view.py index 0f4f1c05..256c6caf 100644 --- a/src/apm_cli/commands/view.py +++ b/src/apm_cli/commands/view.py @@ -478,6 +478,15 @@ def view(package: str, field: Optional[str], global_: bool): display_versions(package, logger) return + # --- marketplace ref without explicit field -> show versions --- + from ..marketplace.resolver import parse_marketplace_ref + + marketplace_ref = parse_marketplace_ref(package) + if marketplace_ref is not None: + plugin_name, marketplace_name, _version_spec = marketplace_ref + _display_marketplace_versions(plugin_name, marketplace_name, logger) + return + # --- default: show local metadata --- scope = InstallScope.USER if global_ else InstallScope.PROJECT if global_: diff --git a/src/apm_cli/deps/lockfile.py b/src/apm_cli/deps/lockfile.py index ac4bb98b..af4085da 100644 --- a/src/apm_cli/deps/lockfile.py +++ b/src/apm_cli/deps/lockfile.py @@ -39,6 +39,7 @@ class LockedDependency: discovered_via: Optional[str] = None # Marketplace name (provenance) marketplace_plugin_name: Optional[str] = None # Plugin name in marketplace version_spec: Optional[str] = None # Original user semver range, e.g., "^2.0.0" + resolved_version: Optional[str] = None # Resolved semver (e.g., "1.2.0") def get_unique_key(self) -> str: """Returns unique key for this dependency.""" @@ -87,6 +88,8 @@ def to_dict(self) -> Dict[str, Any]: result["marketplace_plugin_name"] = self.marketplace_plugin_name if self.version_spec: result["version_spec"] = self.version_spec + if self.resolved_version: + result["resolved_version"] = self.resolved_version return result @classmethod @@ -126,6 +129,7 @@ def from_dict(cls, data: Dict[str, Any]) -> "LockedDependency": discovered_via=data.get("discovered_via"), marketplace_plugin_name=data.get("marketplace_plugin_name"), version_spec=data.get("version_spec"), + resolved_version=data.get("resolved_version"), ) @classmethod diff --git a/src/apm_cli/marketplace/resolver.py b/src/apm_cli/marketplace/resolver.py index 2d05b46b..3dc12246 100644 --- a/src/apm_cli/marketplace/resolver.py +++ b/src/apm_cli/marketplace/resolver.py @@ -10,7 +10,7 @@ import logging import re -from typing import Optional, Tuple +from typing import Callable, Optional, Tuple from ..utils.path_security import PathTraversalError, validate_path_segments from .client import fetch_or_cache @@ -222,7 +222,8 @@ def resolve_marketplace_plugin( *, version_spec: Optional[str] = None, auth_resolver: Optional[object] = None, -) -> Tuple[str, MarketplacePlugin]: + warning_handler: Optional[Callable[[str], None]] = None, +) -> Tuple[str, MarketplacePlugin, Optional[str]]: """Resolve a marketplace plugin reference to a canonical string. When the plugin declares ``versions`` entries and a *version_spec* is @@ -236,9 +237,18 @@ def resolve_marketplace_plugin( git ref. ``None`` selects the latest published version when the plugin has ``versions``. auth_resolver: Optional ``AuthResolver`` instance. + warning_handler: Optional callback for security warnings. When + provided, warnings (immutability violations, shadow detections) + are forwarded here instead of being emitted through Python + stdlib logging. Callers typically pass + ``CommandLogger.warning`` so warnings render through the CLI + output system. Returns: - Tuple of (canonical ``owner/repo[#ref]`` string, resolved plugin). + Tuple of (canonical ``owner/repo[#ref]`` string, resolved plugin, + resolved version string or ``None``). The version is only populated + when the plugin declares ``versions`` and the specifier matched a + published entry; raw git refs and unversioned plugins yield ``None``. Raises: MarketplaceNotFoundError: If the marketplace is not registered. @@ -246,6 +256,14 @@ def resolve_marketplace_plugin( MarketplaceFetchError: If the marketplace cannot be fetched. ValueError: If the plugin source cannot be resolved. """ + + def _emit_warning(msg: str) -> None: + """Route warning through handler when available, else stdlib.""" + if warning_handler is not None: + warning_handler(msg) + else: + logger.warning("%s", msg) + source = get_marketplace_by_name(marketplace_name) manifest = fetch_or_cache(source, auth_resolver=auth_resolver) @@ -260,6 +278,8 @@ def resolve_marketplace_plugin( plugin_root=manifest.plugin_root, ) + resolved_version: Optional[str] = None + # ---- Version-aware ref override ---- if plugin.versions: from .version_resolver import is_version_specifier, resolve_version @@ -281,6 +301,7 @@ def resolve_marketplace_plugin( entry = resolve_version(version_spec, plugin.versions) base = canonical.split("#", 1)[0] canonical = f"{base}#{entry.ref}" + resolved_version = entry.version logger.debug( "Resolved version %s (%s) for %s@%s", entry.version, @@ -296,14 +317,16 @@ def resolve_marketplace_plugin( marketplace_name, plugin_name, entry.version, entry.ref, ) if previous_ref is not None: - logger.warning( + _emit_warning( "Version %s of %s@%s ref changed: was '%s', now '%s'. " - "This may indicate a ref swap attack.", - entry.version, - plugin_name, - marketplace_name, - previous_ref, - entry.ref, + "This may indicate a ref swap attack." + % ( + entry.version, + plugin_name, + marketplace_name, + previous_ref, + entry.ref, + ) ) record_version_pin( marketplace_name, plugin_name, entry.version, entry.ref, @@ -328,14 +351,13 @@ def resolve_marketplace_plugin( plugin_name, marketplace_name, auth_resolver=auth_resolver ) for shadow in shadows: - logger.warning( + _emit_warning( "Plugin '%s' also found in marketplace '%s'. " - "Verify you are installing from the intended source.", - plugin_name, - shadow.marketplace_name, + "Verify you are installing from the intended source." + % (plugin_name, shadow.marketplace_name) ) except Exception: # Shadow detection must never break installation logger.debug("Shadow detection failed", exc_info=True) - return canonical, plugin + return canonical, plugin, resolved_version diff --git a/tests/unit/marketplace/test_marketplace_install_integration.py b/tests/unit/marketplace/test_marketplace_install_integration.py index 9cef01c7..4240ddae 100644 --- a/tests/unit/marketplace/test_marketplace_install_integration.py +++ b/tests/unit/marketplace/test_marketplace_install_integration.py @@ -1,5 +1,6 @@ """Tests for the install flow with mocked marketplace resolution.""" +import sys from unittest.mock import MagicMock, patch import pytest @@ -60,3 +61,130 @@ def test_outcome_no_provenance(self): outcome = _ValidationOutcome(valid=[], invalid=[]) assert outcome.marketplace_provenance is None + + +class TestInstallExitCodeOnAllFailed: + """Bug B2: install must exit(1) when ALL packages fail validation.""" + + @patch("apm_cli.commands.install._validate_and_add_packages_to_apm_yml") + @patch("apm_cli.commands.install.InstallLogger") + @patch("apm_cli.commands.install.DiagnosticCollector") + def test_all_failed_exits_nonzero( + self, mock_diag_cls, mock_logger_cls, mock_validate, tmp_path, monkeypatch + ): + """When outcome.all_failed is True, install raises SystemExit(1).""" + from apm_cli.core.command_logger import _ValidationOutcome + + outcome = _ValidationOutcome( + valid=[], + invalid=[("bad-pkg", "not found")], + ) + mock_validate.return_value = ([], outcome) + + mock_logger = MagicMock() + mock_logger_cls.return_value = mock_logger + + # Create minimal apm.yml so pre-flight check passes + import yaml + apm_yml = tmp_path / "apm.yml" + apm_yml.write_text(yaml.dump({ + "name": "test", "version": "0.1.0", + "dependencies": {"apm": []}, + })) + monkeypatch.chdir(tmp_path) + + from click.testing import CliRunner + from apm_cli.commands.install import install + + runner = CliRunner() + result = runner.invoke(install, ["bad-pkg"], catch_exceptions=False) + assert result.exit_code != 0, ( + f"Expected non-zero exit but got {result.exit_code}" + ) + + +class TestVerboseResolvedVersion: + """Bug B3: verbose install shows resolved version when available.""" + + @patch("apm_cli.commands.install._validate_package_exists", return_value=True) + @patch("apm_cli.commands.install._rich_success") + @patch("apm_cli.marketplace.resolver.resolve_marketplace_plugin") + @patch("apm_cli.marketplace.resolver.parse_marketplace_ref") + def test_resolved_version_logged( + self, mock_parse, mock_resolve, mock_success, mock_validate, + tmp_path, monkeypatch, + ): + """When resolved_version is set, verbose_detail shows it.""" + import yaml + + mock_parse.return_value = ("developer", "agent-forge", "^1.0.0") + mock_resolve.return_value = ( + "acme-org/agent-forge/agents/developer#abc123", + MagicMock(), # resolved_plugin + "1.2.0", # resolved_version + ) + + apm_yml = tmp_path / "apm.yml" + apm_yml.write_text(yaml.dump({ + "name": "test", "version": "0.1.0", + "dependencies": {"apm": []}, + })) + monkeypatch.chdir(tmp_path) + + logger = MagicMock() + logger.verbose = True + + from apm_cli.commands.install import _validate_and_add_packages_to_apm_yml + + _validate_and_add_packages_to_apm_yml( + ["developer@agent-forge#^1.0.0"], + logger=logger, + ) + + # Check verbose_detail was called with the resolved version + calls = [str(c) for c in logger.verbose_detail.call_args_list] + version_calls = [c for c in calls if "Resolved version: 1.2.0" in c] + assert len(version_calls) == 1, ( + f"Expected one 'Resolved version: 1.2.0' call, got: {calls}" + ) + + @patch("apm_cli.commands.install._validate_package_exists", return_value=True) + @patch("apm_cli.commands.install._rich_success") + @patch("apm_cli.marketplace.resolver.resolve_marketplace_plugin") + @patch("apm_cli.marketplace.resolver.parse_marketplace_ref") + def test_no_resolved_version_skips_log( + self, mock_parse, mock_resolve, mock_success, mock_validate, + tmp_path, monkeypatch, + ): + """When resolved_version is None, no version line is logged.""" + import yaml + + mock_parse.return_value = ("developer", "agent-forge", None) + mock_resolve.return_value = ( + "acme-org/agent-forge/agents/developer#main", + MagicMock(), # resolved_plugin + None, # no resolved_version + ) + + apm_yml = tmp_path / "apm.yml" + apm_yml.write_text(yaml.dump({ + "name": "test", "version": "0.1.0", + "dependencies": {"apm": []}, + })) + monkeypatch.chdir(tmp_path) + + logger = MagicMock() + logger.verbose = True + + from apm_cli.commands.install import _validate_and_add_packages_to_apm_yml + + _validate_and_add_packages_to_apm_yml( + ["developer@agent-forge"], + logger=logger, + ) + + calls = [str(c) for c in logger.verbose_detail.call_args_list] + version_calls = [c for c in calls if "Resolved version:" in c] + assert len(version_calls) == 0, ( + f"Expected no 'Resolved version' call, got: {calls}" + ) diff --git a/tests/unit/marketplace/test_shadow_detector.py b/tests/unit/marketplace/test_shadow_detector.py index 89cf0425..fc5cdf2d 100644 --- a/tests/unit/marketplace/test_shadow_detector.py +++ b/tests/unit/marketplace/test_shadow_detector.py @@ -246,7 +246,7 @@ def test_shadow_detection_in_resolver(self, caplog): ) as mock_detect, caplog.at_level(logging.WARNING, logger="apm_cli.marketplace.resolver"), ): - canonical, resolved = resolve_marketplace_plugin( + canonical, resolved, _ = resolve_marketplace_plugin( "sec-check", "acme" ) diff --git a/tests/unit/marketplace/test_versioned_resolver.py b/tests/unit/marketplace/test_versioned_resolver.py index 97e5c748..0cf2d4cd 100644 --- a/tests/unit/marketplace/test_versioned_resolver.py +++ b/tests/unit/marketplace/test_versioned_resolver.py @@ -151,37 +151,42 @@ def _resolve(self, plugin, version_spec=None): def test_caret_spec_resolves_highest_match(self): """^2.0.0 should pick 2.1.0 (highest in ^2 range).""" plugin = _make_versioned_plugin() - canonical, resolved = self._resolve(plugin, version_spec="^2.0.0") + canonical, resolved, resolved_version = self._resolve(plugin, version_spec="^2.0.0") assert canonical == "acme-org/my-plugin#abc123def" assert resolved.name == "my-plugin" + assert resolved_version == "2.1.0" def test_exact_version_spec(self): """Exact version 1.1.0 should pick exactly v1.1.0.""" plugin = _make_versioned_plugin() - canonical, _ = self._resolve(plugin, version_spec="1.1.0") + canonical, _, resolved_version = self._resolve(plugin, version_spec="1.1.0") assert canonical == "acme-org/my-plugin#v1.1.0" + assert resolved_version == "1.1.0" def test_tilde_spec(self): """~1.0.0 should pick 1.1.0 (highest in ~1.0 range: >=1.0.0, <1.1.0). Wait -- tilde means >=1.0.0, <1.1.0. So 1.0.0 is the only match.""" plugin = _make_versioned_plugin() - canonical, _ = self._resolve(plugin, version_spec="~1.0.0") + canonical, _, resolved_version = self._resolve(plugin, version_spec="~1.0.0") assert canonical == "acme-org/my-plugin#v1.0.0" + assert resolved_version == "1.0.0" def test_no_spec_selects_latest(self): """No version_spec (None) selects the highest available version.""" plugin = _make_versioned_plugin() - canonical, _ = self._resolve(plugin, version_spec=None) + canonical, _, resolved_version = self._resolve(plugin, version_spec=None) # 2.1.0 is the highest -> ref = abc123def assert canonical == "acme-org/my-plugin#abc123def" + assert resolved_version == "2.1.0" def test_source_ref_overridden_by_version(self): """The source.ref (main) should be replaced by the version entry ref.""" plugin = _make_versioned_plugin(source_ref="main") - canonical, _ = self._resolve(plugin, version_spec="1.0.0") + canonical, _, resolved_version = self._resolve(plugin, version_spec="1.0.0") # Source had #main, but version resolution should override to v1.0.0 assert canonical == "acme-org/my-plugin#v1.0.0" assert "#main" not in canonical + assert resolved_version == "1.0.0" def test_no_matching_version_raises(self): """Specifier that matches nothing raises ValueError.""" @@ -192,8 +197,9 @@ def test_no_matching_version_raises(self): def test_raw_git_ref_with_versions(self): """A raw git ref (not semver) overrides the canonical ref directly.""" plugin = _make_versioned_plugin(source_ref="main") - canonical, _ = self._resolve(plugin, version_spec="feature-branch") + canonical, _, resolved_version = self._resolve(plugin, version_spec="feature-branch") assert canonical == "acme-org/my-plugin#feature-branch" + assert resolved_version is None class TestResolveMarketplacePluginUnversioned: @@ -222,22 +228,25 @@ def _resolve(self, plugin, version_spec=None): def test_unversioned_no_spec(self): """Unversioned plugin without spec uses source.ref.""" plugin = _make_unversioned_plugin() - canonical, resolved = self._resolve(plugin) + canonical, resolved, resolved_version = self._resolve(plugin) assert canonical == "acme-org/legacy#v1.0" assert resolved.name == "legacy-plugin" + assert resolved_version is None def test_unversioned_with_spec_ignored(self): """Unversioned plugin ignores version_spec -- uses source.ref.""" plugin = _make_unversioned_plugin() - canonical, _ = self._resolve(plugin, version_spec="^2.0.0") + canonical, _, resolved_version = self._resolve(plugin, version_spec="^2.0.0") # No versions on plugin, so version_spec is silently ignored assert canonical == "acme-org/legacy#v1.0" + assert resolved_version is None def test_unversioned_raw_ref_ignored(self): """Unversioned plugin ignores raw ref -- uses source.ref.""" plugin = _make_unversioned_plugin() - canonical, _ = self._resolve(plugin, version_spec="develop") + canonical, _, resolved_version = self._resolve(plugin, version_spec="develop") assert canonical == "acme-org/legacy#v1.0" + assert resolved_version is None # --------------------------------------------------------------------------- @@ -269,10 +278,11 @@ def test_ref_replaces_source_ref(self): return_value=manifest, ), ): - canonical, _ = resolve_marketplace_plugin( + canonical, _, resolved_version = resolve_marketplace_plugin( "my-plugin", "test-mkt", version_spec="3.0.0" ) assert canonical == "org/repo#sha-abc" + assert resolved_version == "3.0.0" def test_source_without_ref_gets_version_ref(self): """When source has no ref, canonical is owner/repo (no #). @@ -296,10 +306,11 @@ def test_source_without_ref_gets_version_ref(self): return_value=manifest, ), ): - canonical, _ = resolve_marketplace_plugin( + canonical, _, resolved_version = resolve_marketplace_plugin( "no-ref-plugin", "test-mkt" ) assert canonical == "org/repo#v1.0.0" + assert resolved_version == "1.0.0" # --------------------------------------------------------------------------- @@ -385,3 +396,211 @@ def test_yaml_lockfile_roundtrip(self): assert dep is not None assert dep.version_spec == "^2.0.0" assert dep.discovered_via == "acme-tools" + + +# --------------------------------------------------------------------------- +# LockedDependency.resolved_version serialization +# --------------------------------------------------------------------------- + + +class TestLockedDependencyResolvedVersion: + """Verify resolved_version field round-trips correctly in LockedDependency.""" + + def test_default_none(self): + dep = LockedDependency(repo_url="owner/repo") + assert dep.resolved_version is None + + def test_to_dict_omits_none(self): + dep = LockedDependency(repo_url="owner/repo") + d = dep.to_dict() + assert "resolved_version" not in d + + def test_to_dict_includes_value(self): + dep = LockedDependency(repo_url="owner/repo", resolved_version="2.1.0") + d = dep.to_dict() + assert d["resolved_version"] == "2.1.0" + + def test_from_dict_missing_field(self): + """Old lockfiles without resolved_version still deserialize.""" + dep = LockedDependency.from_dict({"repo_url": "owner/repo"}) + assert dep.resolved_version is None + + def test_from_dict_with_field(self): + dep = LockedDependency.from_dict({ + "repo_url": "owner/repo", + "resolved_version": "1.5.0", + }) + assert dep.resolved_version == "1.5.0" + + def test_roundtrip(self): + original = LockedDependency( + repo_url="owner/repo", + resolved_commit="abc123", + resolved_ref="v2.1.0", + discovered_via="acme-tools", + marketplace_plugin_name="my-plugin", + version_spec="^2.0.0", + resolved_version="2.1.0", + ) + restored = LockedDependency.from_dict(original.to_dict()) + assert restored.resolved_version == "2.1.0" + assert restored.version_spec == "^2.0.0" + assert restored.discovered_via == "acme-tools" + + def test_backward_compat_existing_fields(self): + """Existing fields still work alongside resolved_version.""" + dep = LockedDependency.from_dict({ + "repo_url": "owner/repo", + "resolved_commit": "abc123", + "content_hash": "sha256:def456", + "is_dev": True, + "discovered_via": "mkt", + "version_spec": ">=1.0.0,<3.0.0", + "resolved_version": "2.0.0", + }) + assert dep.resolved_commit == "abc123" + assert dep.content_hash == "sha256:def456" + assert dep.is_dev is True + assert dep.discovered_via == "mkt" + assert dep.version_spec == ">=1.0.0,<3.0.0" + assert dep.resolved_version == "2.0.0" + + def test_yaml_lockfile_roundtrip(self): + """resolved_version survives a full YAML lockfile write/read cycle.""" + from apm_cli.deps.lockfile import LockFile + + lock = LockFile() + lock.add_dependency(LockedDependency( + repo_url="owner/repo", + version_spec="^2.0.0", + resolved_version="2.1.0", + discovered_via="acme-tools", + )) + + yaml_str = lock.to_yaml() + restored = LockFile.from_yaml(yaml_str) + dep = restored.get_dependency("owner/repo") + assert dep is not None + assert dep.resolved_version == "2.1.0" + assert dep.version_spec == "^2.0.0" + assert dep.discovered_via == "acme-tools" + + +# --------------------------------------------------------------------------- +# B7: warning_handler callback +# --------------------------------------------------------------------------- + + +class TestWarningHandler: + """Verify resolve_marketplace_plugin routes security warnings to handler.""" + + def test_immutability_warning_via_handler(self): + """Ref-swap warning goes through warning_handler, not stdlib.""" + plugin = _make_versioned_plugin() + manifest = _make_manifest(plugin) + source = _make_source() + + captured = [] + + with ( + patch( + "apm_cli.marketplace.resolver.get_marketplace_by_name", + return_value=source, + ), + patch( + "apm_cli.marketplace.resolver.fetch_or_cache", + return_value=manifest, + ), + patch( + "apm_cli.marketplace.version_pins.check_version_pin", + return_value="old-ref-abc", # pretend ref changed + ), + patch( + "apm_cli.marketplace.version_pins.record_version_pin", + ), + ): + resolve_marketplace_plugin( + "my-plugin", + "test-mkt", + warning_handler=captured.append, + ) + + # Exactly one immutability warning + assert len(captured) == 1 + assert "ref changed" in captured[0] + assert "ref swap attack" in captured[0] + assert "my-plugin" in captured[0] + + def test_shadow_warning_via_handler(self): + """Shadow detection warning goes through warning_handler.""" + # Unversioned plugin so we skip version pin logic + plugin = _make_unversioned_plugin() + manifest = _make_manifest(plugin) + source = _make_source() + + captured = [] + + # Shadow mock + from unittest.mock import MagicMock + shadow = MagicMock() + shadow.marketplace_name = "evil-mkt" + + with ( + patch( + "apm_cli.marketplace.resolver.get_marketplace_by_name", + return_value=source, + ), + patch( + "apm_cli.marketplace.resolver.fetch_or_cache", + return_value=manifest, + ), + patch( + "apm_cli.marketplace.shadow_detector.detect_shadows", + return_value=[shadow], + ), + ): + resolve_marketplace_plugin( + "legacy-plugin", + "test-mkt", + warning_handler=captured.append, + ) + + assert len(captured) == 1 + assert "evil-mkt" in captured[0] + assert "legacy-plugin" in captured[0] + + def test_no_handler_falls_back_to_stdlib(self, caplog): + """Without warning_handler, warnings go through Python logging.""" + import logging + + plugin = _make_versioned_plugin() + manifest = _make_manifest(plugin) + source = _make_source() + + with ( + patch( + "apm_cli.marketplace.resolver.get_marketplace_by_name", + return_value=source, + ), + patch( + "apm_cli.marketplace.resolver.fetch_or_cache", + return_value=manifest, + ), + patch( + "apm_cli.marketplace.version_pins.check_version_pin", + return_value="old-ref", + ), + patch( + "apm_cli.marketplace.version_pins.record_version_pin", + ), + caplog.at_level(logging.WARNING, logger="apm_cli.marketplace.resolver"), + ): + resolve_marketplace_plugin( + "my-plugin", + "test-mkt", + # No warning_handler -- should use stdlib logging + ) + + warnings = [r for r in caplog.records if r.levelno == logging.WARNING] + assert len(warnings) >= 1 + assert "ref changed" in warnings[0].message diff --git a/tests/unit/test_outdated_marketplace.py b/tests/unit/test_outdated_marketplace.py index 0b66b7a0..f3990007 100644 --- a/tests/unit/test_outdated_marketplace.py +++ b/tests/unit/test_outdated_marketplace.py @@ -420,6 +420,198 @@ def test_no_version_spec_plain_display(self, mock_get_mkt, mock_fetch): assert "outside range" not in latest +# --------------------------------------------------------------------------- +# Tests: resolved_version fallback (B6) +# --------------------------------------------------------------------------- + +class TestResolvedVersionFallback: + """Tests for resolved_version priority in current version detection.""" + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_resolved_version_used_when_available( + self, mock_get_mkt, mock_fetch, + ): + """resolved_version takes priority over dep.version.""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("skill-auth", ["1.2.0", "2.0.0"])], + ) + dep = _marketplace_dep(version="1.0.0") + # resolved_version is more accurate (e.g., resolved from ^1.0.0) + dep.resolved_version = "1.2.0" # type: ignore[attr-defined] + + result = _check_marketplace_versions(dep, verbose=False) + + assert result is not None + _, current, latest, status, _, _ = result + # Should use resolved_version, not dep.version + assert current == "1.2.0" + assert latest == "2.0.0" + assert status == "outdated" + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_falls_back_to_version_when_no_resolved( + self, mock_get_mkt, mock_fetch, + ): + """Without resolved_version, falls back to dep.version.""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("skill-auth", ["2.1.0", "3.0.0"])], + ) + dep = _marketplace_dep(version="2.1.0") + # No resolved_version attribute set + + result = _check_marketplace_versions(dep, verbose=False) + + assert result is not None + _, current, _, _, _, _ = result + assert current == "2.1.0" + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_version_spec_regex_extraction(self, mock_get_mkt, mock_fetch): + """Extracts base version from version_spec via regex when version is None.""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("skill-auth", ["1.0.0", "2.0.0"])], + ) + dep = _marketplace_dep(version=None) + dep.version_spec = "^1.0.0" # type: ignore[attr-defined] + + result = _check_marketplace_versions(dep, verbose=False) + + assert result is not None + _, current, latest, status, _, _ = result + assert current == "1.0.0" + assert status == "outdated" + # 2.0.0 is outside ^1.0.0 range, so annotation is expected + assert "outside range" in latest + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_compound_version_spec_extraction(self, mock_get_mkt, mock_fetch): + """Extracts first version from compound spec like >=1.0.0,<2.0.0.""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("skill-auth", ["1.0.0", "1.5.0", "2.0.0"])], + ) + dep = _marketplace_dep(version=None) + dep.version_spec = ">=1.0.0,<2.0.0" # type: ignore[attr-defined] + + result = _check_marketplace_versions(dep, verbose=False) + + assert result is not None + _, current, _, status, _, _ = result + # Should extract "1.0.0" from ">=1.0.0,<2.0.0" + assert current == "1.0.0" + assert status == "outdated" + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_no_version_no_spec_returns_none(self, mock_get_mkt, mock_fetch): + """Returns None when neither version nor version_spec available.""" + dep = _marketplace_dep(version=None) + # version_spec defaults to None on LockedDependency + + result = _check_marketplace_versions(dep, verbose=False) + assert result is None + + +# --------------------------------------------------------------------------- +# Tests: best-in-range display (B5) +# --------------------------------------------------------------------------- + +class TestBestInRange: + """Tests for showing best upgrade within version range.""" + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_shows_best_in_range_when_available( + self, mock_get_mkt, mock_fetch, + ): + """When latest is outside range but upgrades exist within, show both.""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin( + "skill-auth", ["1.0.0", "1.2.0", "1.5.0", "2.0.0"], + )], + ) + dep = _marketplace_dep(version="1.0.0") + dep.version_spec = "^1.0.0" # type: ignore[attr-defined] + + result = _check_marketplace_versions(dep, verbose=False) + + assert result is not None + _, current, latest, status, _, _ = result + assert current == "1.0.0" + assert status == "outdated" + # 2.0.0 is outside ^1.0.0, but 1.5.0 is the best within range + assert "outside range ^1.0.0" in latest + assert "best in range: 1.5.0" in latest + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_no_best_in_range_when_already_at_max( + self, mock_get_mkt, mock_fetch, + ): + """When at highest within range, no best-in-range annotation.""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("skill-auth", ["1.5.0", "2.0.0"])], + ) + dep = _marketplace_dep(version="1.5.0") + dep.version_spec = "^1.0.0" # type: ignore[attr-defined] + + result = _check_marketplace_versions(dep, verbose=False) + + assert result is not None + _, _, latest, status, _, _ = result + assert status == "outdated" + assert "outside range ^1.0.0" in latest + assert "best in range" not in latest + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_best_in_range_picks_highest(self, mock_get_mkt, mock_fetch): + """Best-in-range is the highest valid version, not just any.""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin( + "skill-auth", ["1.0.0", "1.1.0", "1.3.0", "1.9.0", "2.0.0"], + )], + ) + dep = _marketplace_dep(version="1.0.0") + dep.version_spec = "^1.0.0" # type: ignore[attr-defined] + + result = _check_marketplace_versions(dep, verbose=False) + + assert result is not None + _, _, latest, _, _, _ = result + # Should pick 1.9.0 as best in range, not 1.1.0 or 1.3.0 + assert "best in range: 1.9.0" in latest + + @patch(_PATCH_FETCH) + @patch(_PATCH_GET_MKT) + def test_latest_within_range_no_annotation(self, mock_get_mkt, mock_fetch): + """When latest version IS within range, no outside-range annotation.""" + mock_get_mkt.return_value = _make_source() + mock_fetch.return_value = _make_manifest( + plugins=[_make_plugin("skill-auth", ["1.0.0", "1.5.0"])], + ) + dep = _marketplace_dep(version="1.0.0") + dep.version_spec = "^1.0.0" # type: ignore[attr-defined] + + result = _check_marketplace_versions(dep, verbose=False) + + assert result is not None + _, _, latest, status, _, _ = result + assert status == "outdated" + assert latest == "1.5.0" + assert "outside range" not in latest + + # --------------------------------------------------------------------------- # Tests: result tuple shape # --------------------------------------------------------------------------- diff --git a/tests/unit/test_view_command.py b/tests/unit/test_view_command.py index 83b0f548..67e549cd 100644 --- a/tests/unit/test_view_command.py +++ b/tests/unit/test_view_command.py @@ -395,3 +395,129 @@ def test_info_alias_hidden_from_help(self): stripped = line.strip() if stripped.startswith("info "): pytest.fail(f"'info' should be hidden but found in help: {line}") + + +# ------------------------------------------------------------------ +# B4: ``apm view plugin@marketplace`` without ``versions`` field +# ------------------------------------------------------------------ + + +class TestViewMarketplaceNoField(_InfoCmdBase): + """``apm view plugin@marketplace`` (no field) shows marketplace versions.""" + + def test_marketplace_ref_shows_versions(self): + """``apm view plugin@mkt`` routes to _display_marketplace_versions.""" + from apm_cli.marketplace.models import ( + MarketplaceManifest, + MarketplacePlugin, + MarketplaceSource, + VersionEntry, + ) + + plugin = MarketplacePlugin( + name="my-plugin", + source={"type": "github", "repo": "acme/plugin"}, + versions=( + VersionEntry(version="1.0.0", ref="abc1234"), + VersionEntry(version="2.0.0", ref="def5678"), + ), + ) + manifest = MarketplaceManifest(name="acme-tools", plugins=(plugin,)) + source = MarketplaceSource(name="acme-tools", owner="acme", repo="marketplace") + + with patch( + "apm_cli.marketplace.registry.get_marketplace_by_name", + return_value=source, + ), patch( + "apm_cli.marketplace.client.fetch_or_cache", + return_value=manifest, + ): + with _force_rich_fallback(): + result = self.runner.invoke( + cli, ["view", "my-plugin@acme-tools"] + ) + + assert result.exit_code == 0 + assert "my-plugin" in result.output + assert "acme-tools" in result.output + assert "2.0.0" in result.output + assert "1.0.0" in result.output + + def test_marketplace_ref_does_not_require_apm_modules(self): + """``apm view plugin@mkt`` works without apm_modules/ directory.""" + from apm_cli.marketplace.models import ( + MarketplaceManifest, + MarketplacePlugin, + MarketplaceSource, + VersionEntry, + ) + + plugin = MarketplacePlugin( + name="my-plugin", + source={"type": "github", "repo": "acme/plugin"}, + versions=(VersionEntry(version="1.0.0", ref="abc1234"),), + ) + manifest = MarketplaceManifest(name="acme-tools", plugins=(plugin,)) + source = MarketplaceSource(name="acme-tools", owner="acme", repo="marketplace") + + with self._chdir_tmp(): + # No apm_modules/ -- should still succeed + with patch( + "apm_cli.marketplace.registry.get_marketplace_by_name", + return_value=source, + ), patch( + "apm_cli.marketplace.client.fetch_or_cache", + return_value=manifest, + ): + with _force_rich_fallback(): + result = self.runner.invoke( + cli, ["view", "my-plugin@acme-tools"] + ) + + assert result.exit_code == 0 + assert "1.0.0" in result.output + + def test_non_marketplace_ref_still_uses_local_path(self): + """``apm view org/repo`` still falls through to local metadata lookup.""" + with self._chdir_tmp() as tmp: + self._make_package( + tmp, "myorg", "myrepo", version="3.0.0", + ) + os.chdir(tmp) + with _force_rich_fallback(): + result = self.runner.invoke(cli, ["view", "myorg/myrepo"]) + + assert result.exit_code == 0 + assert "3.0.0" in result.output + + def test_marketplace_ref_with_version_fragment(self): + """``apm view plugin@mkt#^1.0.0`` (no field) shows versions.""" + from apm_cli.marketplace.models import ( + MarketplaceManifest, + MarketplacePlugin, + MarketplaceSource, + VersionEntry, + ) + + plugin = MarketplacePlugin( + name="my-plugin", + source={"type": "github", "repo": "acme/plugin"}, + versions=(VersionEntry(version="1.0.0", ref="v1.0.0"),), + ) + manifest = MarketplaceManifest(name="acme-tools", plugins=(plugin,)) + source = MarketplaceSource(name="acme-tools", owner="acme", repo="marketplace") + + with patch( + "apm_cli.marketplace.registry.get_marketplace_by_name", + return_value=source, + ), patch( + "apm_cli.marketplace.client.fetch_or_cache", + return_value=manifest, + ): + with _force_rich_fallback(): + result = self.runner.invoke( + cli, ["view", "my-plugin@acme-tools"] + ) + + assert result.exit_code == 0 + assert "1.0.0" in result.output From d500d7946063f180137364c772988b44906b5d45 Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Sat, 11 Apr 2026 20:22:02 +0200 Subject: [PATCH 7/8] =?UTF-8?q?fix:=20address=20Copilot=20review=20feedbac?= =?UTF-8?q?k=20=E2=80=94=20docs=20accuracy,=20path=20security,=20crash=20g?= =?UTF-8?q?uard?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix docs claiming #ref overrides unversioned plugins (it does not) - Fix docs claiming git tag fallback for plugins without versions - Remove -p alias from docs (not in CLI implementation) - Add resolved_version to lockfile-spec documentation - Add --plugin flag to skill file command reference - Remove unused PluginNotFoundError import in view.py - Guard caret pin hint against non-semver version entries - Wrap _expand_specifier in try/except for raw git ref version_specs - Add path traversal protection via ensure_path_within for source.path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/src/content/docs/guides/marketplaces.md | 6 +++--- docs/src/content/docs/reference/cli-commands.md | 4 ++-- docs/src/content/docs/reference/lockfile-spec.md | 1 + packages/apm-guide/.apm/skills/apm-usage/commands.md | 2 +- src/apm_cli/commands/marketplace.py | 4 ++++ src/apm_cli/commands/outdated.py | 5 ++++- src/apm_cli/commands/view.py | 11 ++++++----- 7 files changed, 21 insertions(+), 12 deletions(-) diff --git a/docs/src/content/docs/guides/marketplaces.md b/docs/src/content/docs/guides/marketplaces.md index 1b0a9a3a..2be386f8 100644 --- a/docs/src/content/docs/guides/marketplaces.md +++ b/docs/src/content/docs/guides/marketplaces.md @@ -162,9 +162,9 @@ apm install code-review@acme-plugins#~2.1.0 apm install code-review@acme-plugins#>=1.0.0,<3.0.0 ``` -The `#` separator carries a version specifier when the plugin declares `versions`, or a raw git ref when it does not. Plugins without `versions` continue to work as before. +The `#` separator carries a version specifier only when the plugin declares `versions`. For plugins without `versions`, APM uses the source defined in the marketplace manifest, including any `source.ref` value; `#` does not override unversioned entries. -APM resolves the plugin name against the marketplace index, fetches the underlying Git repository at the resolved ref, and installs it as a standard APM dependency. The resolved source appears in `apm.yml` and `apm.lock.yaml` just like any direct dependency. +APM resolves the plugin name against the marketplace index, fetches the underlying Git repository using the ref defined by the selected marketplace entry, and installs it as a standard APM dependency. The resolved source appears in `apm.yml` and `apm.lock.yaml` just like any direct dependency. For full `apm install` options, see [CLI Commands](../../reference/cli-commands/). @@ -176,7 +176,7 @@ Show available versions for a marketplace plugin: apm view code-review@acme-plugins ``` -Displays a table of versions with their refs, sorted newest-first. For plugins without `versions`, shows remote tags and branches. +Displays a table of versions with their refs, sorted newest-first. Plugins without `versions` show a "no version history" message. ## Provenance tracking diff --git a/docs/src/content/docs/reference/cli-commands.md b/docs/src/content/docs/reference/cli-commands.md index a279e47c..508e7a9a 100644 --- a/docs/src/content/docs/reference/cli-commands.md +++ b/docs/src/content/docs/reference/cli-commands.md @@ -653,7 +653,7 @@ apm view microsoft/apm-sample-package -g - Shows package name, version, description, source, install path, context files, workflows, and hooks - `versions` lists remote tags and branches without cloning the repository - `versions` does not require the package to be installed locally -- `NAME@MARKETPLACE` syntax shows the plugin's declared `versions` array sorted newest-first; for plugins without `versions`, falls back to remote tags and branches +- `NAME@MARKETPLACE` syntax shows the plugin's declared `versions` array sorted newest-first; plugins without `versions` show no version history ### `apm outdated` - Check locked dependencies for updates @@ -1079,7 +1079,7 @@ apm marketplace publish [OPTIONS] **Options:** - `-m, --marketplace TEXT` - Target marketplace name -- `-p, --plugin TEXT` - Plugin name in the marketplace (default: `name` from `apm.yml`) +- `--plugin TEXT` - Plugin name in the marketplace (default: `name` from `apm.yml`) - `--version TEXT` - Version to publish as semver `X.Y.Z` (default: `version` from `apm.yml`) - `--ref TEXT` - Git ref or commit SHA (default: current HEAD) - `--force` - Overwrite existing version entry with a different ref diff --git a/docs/src/content/docs/reference/lockfile-spec.md b/docs/src/content/docs/reference/lockfile-spec.md index 3ea274af..edfd27fc 100644 --- a/docs/src/content/docs/reference/lockfile-spec.md +++ b/docs/src/content/docs/reference/lockfile-spec.md @@ -126,6 +126,7 @@ fields: | `deployed_files` | array of strings | MUST | Every file path APM deployed for this dependency, relative to project root. | | `source` | string | MAY | Dependency source. `"local"` for local path dependencies. Omitted for remote (git) dependencies. | | `version_spec` | string | MAY | Original semver range from the install specifier (e.g., `"^2.0.0"`). Present only for marketplace dependencies installed with a version constraint. Used by `apm outdated` to evaluate updates within the pinned range. | +| `resolved_version` | string | MAY | Concrete version selected after marketplace semver resolution (e.g., `"2.3.1"`). Present only when APM resolved a marketplace dependency from a version constraint. Omitted for raw git refs and unversioned plugins. | | `local_path` | string | MAY | Filesystem path (relative or absolute) to the local package. Present only when `source` is `"local"`. | Fields with empty or default values (empty strings, `false` booleans, empty diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index 4820e204..78d3a1af 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -57,7 +57,7 @@ | `apm marketplace browse NAME` | Browse marketplace packages | -- | | `apm marketplace update [NAME]` | Update marketplace index | -- | | `apm marketplace remove NAME` | Remove a marketplace | `-y` skip confirm | -| `apm marketplace publish` | Publish version to marketplace.json | `-m MARKETPLACE`, `--version`, `--ref`, `--force`, `--dry-run` | +| `apm marketplace publish` | Publish version to marketplace.json | `-m MARKETPLACE`, `--plugin`, `--version`, `--ref`, `--force`, `--dry-run` | | `apm marketplace validate NAME` | Validate marketplace manifest | `--check-refs`, `-v` | | `apm search QUERY@MARKETPLACE` | Search marketplace | `--limit N` | | `apm install NAME@MKT#^X.Y.Z` | Install with semver range | Supports `^`, `~`, `>=`, exact | diff --git a/src/apm_cli/commands/marketplace.py b/src/apm_cli/commands/marketplace.py index b7026e05..8daaa3a3 100644 --- a/src/apm_cli/commands/marketplace.py +++ b/src/apm_cli/commands/marketplace.py @@ -895,6 +895,10 @@ def publish(marketplace_name, version_str, ref, plugin_name, dry_run, force, ver marketplace_file = os.path.join(local_repo, source.path) + # Validate path to prevent traversal attacks via malicious source.path + from ..utils.path_security import ensure_path_within + ensure_path_within(Path(marketplace_file), Path(local_repo)) + if not os.path.isfile(marketplace_file): logger.error( f"marketplace.json not found at expected path: " diff --git a/src/apm_cli/commands/outdated.py b/src/apm_cli/commands/outdated.py index f31cca1a..0e77e749 100644 --- a/src/apm_cli/commands/outdated.py +++ b/src/apm_cli/commands/outdated.py @@ -143,7 +143,10 @@ def _check_marketplace_versions(dep, verbose): # Check version_spec if available (field added by parallel task) version_spec = getattr(dep, "version_spec", None) if version_spec: - constraints = _expand_specifier(version_spec) + try: + constraints = _expand_specifier(version_spec) + except ValueError: + constraints = None if constraints and not _version_matches(best_parsed, constraints): # Find best version within the range best_in_range = None diff --git a/src/apm_cli/commands/view.py b/src/apm_cli/commands/view.py index 256c6caf..9383e0e3 100644 --- a/src/apm_cli/commands/view.py +++ b/src/apm_cli/commands/view.py @@ -253,7 +253,7 @@ def _display_marketplace_versions( Fetches the marketplace manifest, finds the plugin, and renders its ``versions[]`` array as a Rich table (with plain-text fallback). """ - from ..marketplace.errors import MarketplaceFetchError, PluginNotFoundError + from ..marketplace.errors import MarketplaceFetchError from ..marketplace.models import MarketplaceSource from ..marketplace.registry import get_marketplace_by_name from ..marketplace.client import fetch_or_cache @@ -334,10 +334,11 @@ def _display_marketplace_versions( console.print(table) click.echo("") click.echo(f" Install: apm install {plugin_name}@{marketplace_name}") - click.echo( - f" Pin: apm install {plugin_name}@{marketplace_name}" - f"#^{sorted_versions[0].version}" - ) + if latest_version: + click.echo( + f" Pin: apm install {plugin_name}@{marketplace_name}" + f"#^{latest_version}" + ) except ImportError: # Plain-text fallback From 0517b3eb9ae02978ed7fa32dcf128a1e735cff05 Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Sat, 11 Apr 2026 20:49:21 +0200 Subject: [PATCH 8/8] fix: guard caret pin hint in plain-text fallback path The Rich rendering path already guarded the caret pin hint with 'if latest_version:', but the plain-text fallback still used sorted_versions[0].version unconditionally, which could produce invalid specifiers like '#^nightly' for non-semver entries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/commands/view.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/apm_cli/commands/view.py b/src/apm_cli/commands/view.py index 9383e0e3..a6a53dc9 100644 --- a/src/apm_cli/commands/view.py +++ b/src/apm_cli/commands/view.py @@ -353,10 +353,11 @@ def _display_marketplace_versions( ) click.echo("") click.echo(f" Install: apm install {plugin_name}@{marketplace_name}") - click.echo( - f" Pin: apm install {plugin_name}@{marketplace_name}" - f"#^{sorted_versions[0].version}" - ) + if latest_version: + click.echo( + f" Pin: apm install {plugin_name}@{marketplace_name}" + f"#^{latest_version}" + ) def display_versions(package: str, logger: CommandLogger) -> None: