fix(marketplace): add honors manifest.name for Claude Code parity

src/apm_cli/commands/marketplace.py

@@ -7,6 +7,7 @@
 import builtins
 import json
 import os
+import re
 import subprocess
 import sys
 import traceback
@@ -44,6 +45,16 @@
 from ._helpers import _get_console, _is_interactive
 
 
+# Marketplace alias must satisfy this pattern so it can appear on the right of
+# ``@`` in ``apm install <plugin>@<marketplace>`` syntax.
+_ALIAS_PATTERN = re.compile(r"^[a-zA-Z0-9._-]+$")
+
+
+def _is_valid_alias(value: str) -> bool:
+    """Return True when ``value`` is a legal marketplace alias."""
+    return bool(value) and _ALIAS_PATTERN.match(value) is not None
+
+
 # ---------------------------------------------------------------------------
 # Custom group for organised --help output
 # ---------------------------------------------------------------------------
@@ -336,28 +347,22 @@ def add(repo, name, branch, host, verbose):
             resolved_host = normalized_host
         else:
             resolved_host = default_host()
-        display_name = name or repo_name
-
-        # Validate name is identifier-compatible for NAME@MARKETPLACE syntax
-        import re
 
-        if not re.match(r"^[a-zA-Z0-9._-]+$", display_name):
+        # Hard-fail if the user-supplied --name flag is malformed; the
+        # manifest's name is validated softly below (publisher mistakes
+        # shouldn't break a successful add).
+        if name is not None and not _is_valid_alias(name):
             logger.error(
-                f"Invalid marketplace name: '{display_name}'. "
+                f"Invalid marketplace name: '{name}'. "
                 f"Names must only contain letters, digits, '.', '_', and '-' "
                 f"(required for 'apm install plugin@marketplace' syntax)."
             )
             sys.exit(1)
 
-        logger.start(f"Registering marketplace '{display_name}'...", symbol="gear")
-        logger.verbose_detail(f"    Repository: {owner}/{repo_name}")
-        logger.verbose_detail(f"    Branch: {branch}")
-        if resolved_host != "github.com":
-            logger.verbose_detail(f"    Host: {resolved_host}")
-
-        # Auto-detect marketplace.json location
+        # Probe for the marketplace.json location. The probe source's name
+        # is a placeholder -- _auto_detect_path only consults host/owner/repo.
         probe_source = MarketplaceSource(
-            name=display_name,
+            name=name or repo_name,
             owner=owner,
             repo=repo_name,
             branch=branch,
@@ -373,9 +378,56 @@ def add(repo, name, branch, host, verbose):
             )
             sys.exit(1)
 
+        # Fetch and validate the manifest before logging start, so that the
+        # success/start lines display the *final* alias the user must use.
+        fetch_source = MarketplaceSource(
+            name=name or repo_name,
+            owner=owner,
+            repo=repo_name,
+            branch=branch,
+            host=resolved_host,
+            path=detected_path,
+        )
+        manifest = fetch_marketplace(fetch_source, force_refresh=True)
+        plugin_count = len(manifest.plugins)
+
+        # Resolve final alias: --name flag > manifest.name (if valid) > repo name.
+        # Track which tier won so we can report it in verbose mode and emit a
+        # warning when a publisher-declared name had to be rejected.
+        manifest_name = (manifest.name or "").strip()
+        if name is not None:
+            display_name = name
+            alias_source = "--name flag"
+        elif manifest_name and _is_valid_alias(manifest_name):
+            display_name = manifest_name
+            alias_source = f"manifest.name ('{manifest_name}')"
+        else:
+            display_name = repo_name
+            if manifest_name and not _is_valid_alias(manifest_name):
+                logger.warning(
+                    f"Manifest declares name '{manifest_name}' which is not a "
+                    f"valid alias (must match [a-zA-Z0-9._-]+). "
+                    f"Falling back to repo name."
+                )
+                alias_source = f"repo name (manifest.name '{manifest_name}' invalid)"
+            else:
+                alias_source = "repo name (manifest.name missing)"
+
+        # Defense-in-depth: repo names from GitHub already satisfy the alias
+        # regex, so this invariant should always hold by the time we register.
+        assert _is_valid_alias(display_name), (
+            f"Resolved marketplace alias '{display_name}' failed validation"
+        )
+
+        logger.start(f"Registering marketplace '{display_name}'...", symbol="gear")
+        logger.verbose_detail(f"    Repository: {owner}/{repo_name}")
+        logger.verbose_detail(f"    Branch: {branch}")
+        if resolved_host != "github.com":
+            logger.verbose_detail(f"    Host: {resolved_host}")
         logger.verbose_detail(f"    Detected path: {detected_path}")
+        logger.verbose_detail(f"    Alias source: {alias_source}")
 
-        # Create source with detected path
+        # Persist with the final alias.
         source = MarketplaceSource(
             name=display_name,
             owner=owner,
@@ -384,12 +436,6 @@ def add(repo, name, branch, host, verbose):
             host=resolved_host,
             path=detected_path,
         )
-
-        # Fetch and validate
-        manifest = fetch_marketplace(source, force_refresh=True)
-        plugin_count = len(manifest.plugins)
-
-        # Register
         add_marketplace(source)
 
         logger.success(
@@ -399,6 +445,14 @@ def add(repo, name, branch, host, verbose):
         if manifest.description:
             logger.verbose_detail(f"    {manifest.description}")
 
+        # Surface the install syntax only when the alias is something the user
+        # could not have predicted from OWNER/REPO. Silence is fine otherwise.
+        if name is None and display_name != repo_name:
+            logger.progress(
+                f"Install plugins with: apm install <plugin>@{display_name}",
+                symbol="info",
+            )
+
     except Exception as e:  # noqa: BLE001 -- top-level command catch-all
         logger.error(f"Failed to register marketplace: {e}")
         if verbose:

tests/unit/marketplace/test_marketplace_commands.py

@@ -38,6 +38,154 @@ def test_invalid_format_no_slash(self, runner):
         assert result.exit_code != 0
         assert "OWNER/REPO" in result.output
 
+    @patch("apm_cli.marketplace.registry.add_marketplace")
+    @patch("apm_cli.marketplace.client.fetch_marketplace")
+    @patch("apm_cli.marketplace.client._auto_detect_path")
+    def test_add_uses_manifest_name_when_available(
+        self, mock_detect, mock_fetch, mock_add, runner
+    ):
+        """Manifest's `name` field becomes the registered alias."""
+        from apm_cli.commands.marketplace import marketplace
+
+        mock_detect.return_value = ".claude-plugin/marketplace.json"
+        mock_fetch.return_value = MarketplaceManifest(
+            name="addy-agent-skills",
+            plugins=(MarketplacePlugin(name="agent-skills"),),
+        )
+
+        result = runner.invoke(marketplace, ["add", "addyosmani/agent-skills"])
+        assert result.exit_code == 0
+        # Registered source carries the manifest's name, not the repo name.
+        registered_source = mock_add.call_args[0][0]
+        assert registered_source.name == "addy-agent-skills"
+        assert registered_source.repo == "agent-skills"
+        # Install hint surfaces the alias the user must use next.
+        assert "apm install <plugin>@addy-agent-skills" in result.output
+
+    @patch("apm_cli.marketplace.registry.add_marketplace")
+    @patch("apm_cli.marketplace.client.fetch_marketplace")
+    @patch("apm_cli.marketplace.client._auto_detect_path")
+    def test_add_cli_name_overrides_manifest(
+        self, mock_detect, mock_fetch, mock_add, runner
+    ):
+        """An explicit --name flag wins over the manifest's name."""
+        from apm_cli.commands.marketplace import marketplace
+
+        mock_detect.return_value = "marketplace.json"
+        mock_fetch.return_value = MarketplaceManifest(
+            name="manifest-alias",
+            plugins=(MarketplacePlugin(name="p1"),),
+        )
+
+        result = runner.invoke(
+            marketplace, ["add", "acme/plugins", "--name", "custom-alias"]
+        )
+        assert result.exit_code == 0
+        registered_source = mock_add.call_args[0][0]
+        assert registered_source.name == "custom-alias"
+        # No install hint when the user explicitly chose the alias.
+        assert "Install plugins with" not in result.output
+
+    @patch("apm_cli.marketplace.registry.add_marketplace")
+    @patch("apm_cli.marketplace.client.fetch_marketplace")
+    @patch("apm_cli.marketplace.client._auto_detect_path")
+    def test_add_falls_back_when_manifest_name_invalid(
+        self, mock_detect, mock_fetch, mock_add, runner
+    ):
+        """Invalid manifest.name triggers a soft fallback to the repo name."""
+        from apm_cli.commands.marketplace import marketplace
+
+        mock_detect.return_value = "marketplace.json"
+        mock_fetch.return_value = MarketplaceManifest(
+            name="has spaces!",
+            plugins=(MarketplacePlugin(name="p1"),),
+        )
+
+        result = runner.invoke(marketplace, ["add", "acme/plugins"])
+        # Soft fallback: the command still succeeds.
+        assert result.exit_code == 0
+        registered_source = mock_add.call_args[0][0]
+        assert registered_source.name == "plugins"
+        # User sees a warning quoting the offending value.
+        assert "has spaces!" in result.output
+        assert "Falling back to repo name" in result.output
+
+    @patch("apm_cli.marketplace.registry.add_marketplace")
+    @patch("apm_cli.marketplace.client.fetch_marketplace")
+    @patch("apm_cli.marketplace.client._auto_detect_path")
+    def test_add_falls_back_when_manifest_name_missing(
+        self, mock_detect, mock_fetch, mock_add, runner
+    ):
+        """Empty manifest.name silently falls back to the repo name."""
+        from apm_cli.commands.marketplace import marketplace
+
+        mock_detect.return_value = "marketplace.json"
+        mock_fetch.return_value = MarketplaceManifest(
+            name="",
+            plugins=(MarketplacePlugin(name="p1"),),
+        )
+
+        result = runner.invoke(marketplace, ["add", "acme/plugins"])
+        assert result.exit_code == 0
+        registered_source = mock_add.call_args[0][0]
+        assert registered_source.name == "plugins"
+        # No warning when the publisher simply omitted the field.
+        assert "Falling back" not in result.output
+        # No install hint either: alias matches the repo name -- predictable.
+        assert "Install plugins with" not in result.output
+
+    def test_add_rejects_invalid_cli_name(self, runner):
+        """An invalid --name flag is a user error and hard-fails."""
+        from apm_cli.commands.marketplace import marketplace
+
+        result = runner.invoke(
+            marketplace, ["add", "acme/plugins", "--name", "bad name"]
+        )
+        assert result.exit_code != 0
+        assert "Invalid marketplace name" in result.output
+
+    @patch("apm_cli.marketplace.registry.add_marketplace")
+    @patch("apm_cli.marketplace.client.fetch_marketplace")
+    @patch("apm_cli.marketplace.client._auto_detect_path")
+    def test_add_awesome_copilot_pattern_unchanged(
+        self, mock_detect, mock_fetch, mock_add, runner
+    ):
+        """Regression: github/awesome-copilot manifest name == repo name -> no behaviour change."""
+        from apm_cli.commands.marketplace import marketplace
+
+        mock_detect.return_value = ".github/plugin/marketplace.json"
+        mock_fetch.return_value = MarketplaceManifest(
+            name="awesome-copilot",
+            plugins=(MarketplacePlugin(name="azure-cloud-development"),),
+        )
+
+        result = runner.invoke(marketplace, ["add", "github/awesome-copilot"])
+        assert result.exit_code == 0
+        registered_source = mock_add.call_args[0][0]
+        assert registered_source.name == "awesome-copilot"
+        # Alias matches the repo name, so the install hint is suppressed.
+        assert "Install plugins with" not in result.output
+
+    @patch("apm_cli.marketplace.client.fetch_marketplace")
+    @patch("apm_cli.marketplace.client._auto_detect_path")
+    def test_add_verbose_shows_alias_source(
+        self, mock_detect, mock_fetch, runner
+    ):
+        """Verbose mode reports which precedence tier picked the alias."""
+        from apm_cli.commands.marketplace import marketplace
+
+        mock_detect.return_value = "marketplace.json"
+        mock_fetch.return_value = MarketplaceManifest(
+            name="acme-tools",
+            plugins=(MarketplacePlugin(name="p1"),),
+        )
+
+        result = runner.invoke(
+            marketplace, ["add", "acme/plugins", "--verbose"]
+        )
+        assert result.exit_code == 0
+        assert "Alias source: manifest.name" in result.output
+
     @patch("apm_cli.marketplace.client.fetch_marketplace")
     @patch("apm_cli.marketplace.client._auto_detect_path")
     def test_successful_add(self, mock_detect, mock_fetch, runner):