fix(install): MARKETPLACE_PLUGIN beats HOOK_PACKAGE in detection cascade (#780)

CHANGELOG.md

@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- `apm install` no longer silently drops skills, agents, and commands when a Claude Code plugin also ships `hooks/*.json`. The package-type detection cascade now classifies plugin-shaped packages as `MARKETPLACE_PLUGIN` (which already maps hooks via the plugin synthesizer) before falling back to the hook-only classification, and emits a default-visibility `[!]` warning when a hook-only classification disagrees with the package's directory contents (#780)
 - Preserve custom git ports across protocols: non-default ports on `ssh://` and `https://` dependency URLs (e.g. Bitbucket Datacenter on SSH port 7999, self-hosted GitLab on HTTPS port 8443) are now captured as a first-class `port` field on `DependencyReference` and threaded through all clone URL builders. When the SSH clone fails, the HTTPS fallback reuses the same port instead of silently dropping it (#661, #731)
 
 ## [0.8.12] - 2026-04-19

src/apm_cli/install/sources.py

@@ -40,6 +40,27 @@
     from apm_cli.models.apm_package import PackageInfo
 
 
+def _format_package_type_label(pkg_type) -> Optional[str]:
+    """Human-readable label for a detected ``PackageType``.
+
+    Centralised so every install path emits the same wording and so
+    new ``PackageType`` values can be added without grepping for ad-hoc
+    dicts.  Missing ``HOOK_PACKAGE`` from this table is what made
+    microsoft/apm#780 silent -- keep all classifiable enum members
+    covered.
+    """
+    from apm_cli.models.apm_package import PackageType
+
+    return {
+        PackageType.CLAUDE_SKILL: "Skill (SKILL.md detected)",
+        PackageType.MARKETPLACE_PLUGIN:
+            "Marketplace Plugin (plugin.json or agents/skills/commands)",
+        PackageType.HYBRID: "Hybrid (apm.yml + SKILL.md)",
+        PackageType.APM_PACKAGE: "APM Package (apm.yml)",
+        PackageType.HOOK_PACKAGE: "Hook Package (hooks/*.json only)",
+    }.get(pkg_type)
+
+
 @dataclass
 class Materialization:
     """Outcome of ``DependencySource.acquire()``.
@@ -510,12 +531,7 @@ def acquire(self) -> Optional[Materialization]:
 
             if hasattr(package_info, "package_type"):
                 package_type = package_info.package_type
-                _type_label = {
-                    PackageType.CLAUDE_SKILL: "Skill (SKILL.md detected)",
-                    PackageType.MARKETPLACE_PLUGIN: "Marketplace Plugin (plugin.json detected)",
-                    PackageType.HYBRID: "Hybrid (apm.yml + SKILL.md)",
-                    PackageType.APM_PACKAGE: "APM Package (apm.yml)",
-                }.get(package_type)
+                _type_label = _format_package_type_label(package_type)
                 if _type_label and logger:
                     logger.package_type_info(_type_label)
 

src/apm_cli/models/validation.py

@@ -127,6 +127,12 @@ def summary(self) -> str:
             return f"[x] Package is invalid with {len(self.errors)} error(s)"
 
 
+# Canonical order of the directories that mark a Claude Code marketplace
+# plugin.  Tests assert this ordering on ``DetectionEvidence.plugin_dirs_present``
+# so adding a new directory here is a public-API change.
+_PLUGIN_DIRS: Tuple[str, ...] = ("agents", "skills", "commands")
+
+
 def _has_hook_json(package_path: Path) -> bool:
     """Check if the package has hook JSON files in hooks/ or .apm/hooks/."""
     for hooks_dir in [package_path / "hooks", package_path / APM_DIR / "hooks"]:
@@ -135,42 +141,110 @@ def _has_hook_json(package_path: Path) -> bool:
     return False
 
 
+@dataclass(frozen=True)
+class DetectionEvidence:
+    """Snapshot of the file-system signals that drove classification.
+
+    Returned from :func:`gather_detection_evidence` and consumed by
+    install-time observability (verbose detection traces, near-miss
+    warnings, deploy-summary labelling).  Kept independent of
+    :func:`detect_package_type` so that the classification function can
+    keep its existing ``(PackageType, Optional[Path])`` return signature
+    while observability code can pull richer detail on demand.
+    """
+
+    has_apm_yml: bool
+    has_skill_md: bool
+    has_hook_json: bool
+    plugin_json_path: Optional[Path]
+    plugin_dirs_present: Tuple[str, ...]
+    has_claude_plugin_dir: bool = False
+
+    @property
+    def has_plugin_evidence(self) -> bool:
+        """True if any signal indicates this is a marketplace plugin.
+
+        ``.claude-plugin/`` is treated as first-class evidence so that a
+        Claude Code plugin without a ``plugin.json`` (name derived from
+        the directory) classifies as ``MARKETPLACE_PLUGIN`` instead of
+        falling through to ``HOOK_PACKAGE``.  ``normalize_plugin_directory``
+        handles the missing-manifest case gracefully.
+        """
+        return (
+            self.plugin_json_path is not None
+            or bool(self.plugin_dirs_present)
+            or self.has_claude_plugin_dir
+        )
+
+
+def gather_detection_evidence(package_path: Path) -> DetectionEvidence:
+    """Collect all package-type signals from a directory in one pass.
+
+    Pure: no side-effects, no file mutations.  Cheap (a handful of stat
+    calls).  See :class:`DetectionEvidence` for the shape of the return
+    value.
+    """
+    from ..utils.helpers import find_plugin_json
+
+    plugin_dirs_present = tuple(
+        name for name in _PLUGIN_DIRS if (package_path / name).is_dir()
+    )
+    return DetectionEvidence(
+        has_apm_yml=(package_path / APM_YML_FILENAME).exists(),
+        has_skill_md=(package_path / SKILL_MD_FILENAME).exists(),
+        has_hook_json=_has_hook_json(package_path),
+        plugin_json_path=find_plugin_json(package_path),
+        plugin_dirs_present=plugin_dirs_present,
+        has_claude_plugin_dir=(package_path / ".claude-plugin").is_dir(),
+    )
+
+
 def detect_package_type(
     package_path: Path,
 ) -> Tuple[PackageType, Optional[Path]]:
     """Classify a package directory into a ``PackageType``.
 
-    This is the **single source of truth** for the detection cascade.
-    The function is pure — no side-effects, no file mutations.
+    Single source of truth for the detection cascade.  Pure: no
+    side-effects, no file mutations.
+
+    Cascade order (first match wins):
+
+    1. ``HYBRID`` -- both ``apm.yml`` and ``SKILL.md`` present.
+    2. ``APM_PACKAGE`` -- ``apm.yml`` only.
+    3. ``CLAUDE_SKILL`` -- ``SKILL.md`` only.
+    4. ``MARKETPLACE_PLUGIN`` -- ``plugin.json``, a ``.claude-plugin/``
+       directory, *or* one of ``agents/``, ``skills/``, ``commands/``.
+       This must precede the hook-only branch because the
+       marketplace-plugin synthesizer (``_map_plugin_artifacts``) already
+       maps ``hooks/`` alongside agents/skills/commands -- so a Claude
+       Code plugin that ships both hooks and skills must classify as
+       ``MARKETPLACE_PLUGIN``, not ``HOOK_PACKAGE``, otherwise the
+       skills are silently dropped.  ``.claude-plugin/`` is treated as
+       first-class evidence so plugins without a ``plugin.json``
+       (manifest-less Claude Code plugins) still classify correctly;
+       ``normalize_plugin_directory`` handles missing manifests.
+       See microsoft/apm#780.
+    5. ``HOOK_PACKAGE`` -- ``hooks/*.json`` only, no plugin evidence.
+    6. ``INVALID`` -- nothing recognisable.
 
     Returns:
-        A ``(package_type, plugin_json_path)`` tuple.
-        *plugin_json_path* is non-None only for ``MARKETPLACE_PLUGIN``.
+        A ``(package_type, plugin_json_path)`` tuple.  *plugin_json_path*
+        is non-None only when ``MARKETPLACE_PLUGIN`` was matched via an
+        actual ``plugin.json`` file (not via directory evidence alone).
     """
-    from ..utils.helpers import find_plugin_json
-
-    has_apm_yml = (package_path / APM_YML_FILENAME).exists()
-    has_skill_md = (package_path / SKILL_MD_FILENAME).exists()
+    evidence = gather_detection_evidence(package_path)
 
-    if has_apm_yml and has_skill_md:
+    if evidence.has_apm_yml and evidence.has_skill_md:
         return PackageType.HYBRID, None
-    if has_apm_yml:
+    if evidence.has_apm_yml:
         return PackageType.APM_PACKAGE, None
-    if has_skill_md:
+    if evidence.has_skill_md:
         return PackageType.CLAUDE_SKILL, None
-    if _has_hook_json(package_path):
+    if evidence.has_plugin_evidence:
+        return PackageType.MARKETPLACE_PLUGIN, evidence.plugin_json_path
+    if evidence.has_hook_json:
         return PackageType.HOOK_PACKAGE, None
 
-    plugin_json_path = find_plugin_json(package_path)
-    has_plugin_evidence = (
-        plugin_json_path is not None
-        or (package_path / "agents").is_dir()
-        or (package_path / "skills").is_dir()
-        or (package_path / "commands").is_dir()
-    )
-    if has_plugin_evidence:
-        return PackageType.MARKETPLACE_PLUGIN, plugin_json_path
-
     return PackageType.INVALID, None
 
 

tests/test_apm_package_models.py

@@ -1089,6 +1089,133 @@ def test_hook_package_apm_yml_precedence(self, tmp_path):
         pkg_type, _ = detect_package_type(tmp_path)
         assert pkg_type == PackageType.APM_PACKAGE
 
+    def test_marketplace_plugin_wins_over_hooks_via_agents_dir(self, tmp_path):
+        """Regression: a Claude plugin that ships hooks AND agents/ must
+        classify as MARKETPLACE_PLUGIN so the plugin synthesizer maps
+        agents alongside hooks. See microsoft/apm#780.
+        """
+        hooks_dir = tmp_path / "hooks"
+        hooks_dir.mkdir()
+        (hooks_dir / "hooks.json").write_text("{}")
+        (tmp_path / "agents").mkdir()
+        pkg_type, pj_path = detect_package_type(tmp_path)
+        assert pkg_type == PackageType.MARKETPLACE_PLUGIN
+        assert pj_path is None
+
+    def test_marketplace_plugin_wins_over_hooks_via_plugin_json(self, tmp_path):
+        """Regression: hooks must not pre-empt classification when a
+        plugin.json is present. See microsoft/apm#780.
+        """
+        hooks_dir = tmp_path / "hooks"
+        hooks_dir.mkdir()
+        (hooks_dir / "hooks.json").write_text("{}")
+        (tmp_path / "plugin.json").write_text('{"name": "test"}')
+        pkg_type, pj_path = detect_package_type(tmp_path)
+        assert pkg_type == PackageType.MARKETPLACE_PLUGIN
+        assert pj_path is not None
+        assert pj_path.name == "plugin.json"
+
+    def test_obra_superpowers_layout(self, tmp_path):
+        """Full-fidelity reproducer for microsoft/apm#780: the
+        obra/superpowers repo ships hooks/hooks.json alongside
+        .claude-plugin/plugin.json + agents/ + skills/ + commands/.
+        Pre-fix, this classified as HOOK_PACKAGE and the skills,
+        agent, and commands were silently dropped.
+        """
+        # Hooks (the trap that was firing first).
+        (tmp_path / "hooks").mkdir()
+        (tmp_path / "hooks" / "hooks.json").write_text("{}")
+        # Plugin shape.
+        (tmp_path / ".claude-plugin").mkdir()
+        (tmp_path / ".claude-plugin" / "plugin.json").write_text(
+            '{"name": "superpowers"}'
+        )
+        (tmp_path / "agents").mkdir()
+        (tmp_path / "agents" / "code-reviewer.md").write_text("# agent")
+        (tmp_path / "skills").mkdir()
+        (tmp_path / "skills" / "tdd").mkdir()
+        (tmp_path / "skills" / "tdd" / "SKILL.md").write_text("# tdd")
+        (tmp_path / "commands").mkdir()
+        (tmp_path / "commands" / "foo.md").write_text("# cmd")
+        pkg_type, pj_path = detect_package_type(tmp_path)
+        assert pkg_type == PackageType.MARKETPLACE_PLUGIN
+        assert pj_path is not None
+        assert pj_path.name == "plugin.json"
+
+
+class TestGatherDetectionEvidence:
+    """Tests for the evidence-gathering helper that powers observability."""
+
+    def test_empty_directory(self, tmp_path):
+        from apm_cli.models.validation import gather_detection_evidence
+
+        evidence = gather_detection_evidence(tmp_path)
+        assert evidence.has_apm_yml is False
+        assert evidence.has_skill_md is False
+        assert evidence.has_hook_json is False
+        assert evidence.plugin_json_path is None
+        assert evidence.plugin_dirs_present == ()
+        assert evidence.has_plugin_evidence is False
+
+    def test_records_plugin_dirs_in_canonical_order(self, tmp_path):
+        from apm_cli.models.validation import gather_detection_evidence
+
+        # Create in non-canonical order; expect canonical order in result.
+        (tmp_path / "commands").mkdir()
+        (tmp_path / "agents").mkdir()
+        (tmp_path / "skills").mkdir()
+        evidence = gather_detection_evidence(tmp_path)
+        assert evidence.plugin_dirs_present == ("agents", "skills", "commands")
+        assert evidence.has_plugin_evidence is True
+
+    def test_obra_superpowers_evidence(self, tmp_path):
+        """Evidence on the #780 repro should expose every signal the
+        UX layer needs to explain the classification.
+        """
+        from apm_cli.models.validation import gather_detection_evidence
+
+        (tmp_path / "hooks").mkdir()
+        (tmp_path / "hooks" / "hooks.json").write_text("{}")
+        (tmp_path / ".claude-plugin").mkdir()
+        (tmp_path / ".claude-plugin" / "plugin.json").write_text(
+            '{"name": "superpowers"}'
+        )
+        (tmp_path / "agents").mkdir()
+        (tmp_path / "skills").mkdir()
+        (tmp_path / "commands").mkdir()
+        evidence = gather_detection_evidence(tmp_path)
+        assert evidence.has_hook_json is True
+        assert evidence.plugin_json_path is not None
+        assert evidence.plugin_dirs_present == ("agents", "skills", "commands")
+        assert evidence.has_claude_plugin_dir is True
+        assert evidence.has_plugin_evidence is True
+
+    def test_claude_plugin_dir_alone_is_plugin_evidence(self, tmp_path):
+        """A bare ``.claude-plugin/`` directory (no plugin.json, no
+        agents/skills/commands) must still classify as plugin evidence
+        so a Claude Code plugin without a manifest is not silently
+        treated as hooks-only.  See microsoft/apm#780.
+        """
+        from src.apm_cli.models.validation import (
+            detect_package_type,
+            gather_detection_evidence,
+        )
+
+        (tmp_path / ".claude-plugin").mkdir()
+        (tmp_path / "hooks").mkdir()
+        (tmp_path / "hooks" / "hooks.json").write_text("{}")
+
+        evidence = gather_detection_evidence(tmp_path)
+        assert evidence.has_claude_plugin_dir is True
+        assert evidence.plugin_dirs_present == ()
+        assert evidence.plugin_json_path is None
+        assert evidence.has_plugin_evidence is True
+
+        pkg_type, pj_path = detect_package_type(tmp_path)
+        assert pkg_type == PackageType.MARKETPLACE_PLUGIN
+        # No plugin.json file present -> path is None even though we matched.
+        assert pj_path is None
+
 
 class TestGitReferenceUtils:
     """Test Git reference parsing utilities."""

tests/unit/install/test_sources_classification.py

@@ -0,0 +1,37 @@
+"""Tests for package-type classification observability in install/sources.
+
+Covers the label table that feeds ``CommandLogger.package_type_info``
+so every classifiable ``PackageType`` has a human-readable label.
+
+Regression suite for microsoft/apm#780.
+"""
+from __future__ import annotations
+
+from apm_cli.install.sources import _format_package_type_label
+from apm_cli.models.apm_package import PackageType
+
+
+class TestFormatPackageTypeLabel:
+    def test_all_classifiable_types_have_labels(self):
+        """Every classifiable PackageType must have a human label.
+
+        Missing entries make classification silent (the bug class behind
+        microsoft/apm#780).  ``INVALID`` is excluded -- it short-circuits
+        installation upstream with a dedicated error path.
+        """
+        for pkg_type in PackageType:
+            if pkg_type == PackageType.INVALID:
+                continue
+            assert _format_package_type_label(pkg_type) is not None, (
+                f"{pkg_type.name} has no human-readable label"
+            )
+
+    def test_hook_package_label_includes_format_hint(self):
+        label = _format_package_type_label(PackageType.HOOK_PACKAGE)
+        assert "hooks/*.json" in label
+
+    def test_marketplace_plugin_label_mentions_dirs(self):
+        """Label must reflect that classification fires on plugin.json
+        OR on agents/skills/commands directories alone."""
+        label = _format_package_type_label(PackageType.MARKETPLACE_PLUGIN)
+        assert "agents" in label and "skills" in label and "commands" in label