fix: install versioned marketplace package directories

.apm/architecture/owners/install-deployment.json

@@ -70,9 +70,9 @@
     },
     {
       "id": "prospective-dry-run-install-plan",
-      "decision": "Prospective dry-run install plan",
-      "owner": "install/dry_run_plan.py (ProspectiveInstallPlan)",
-      "selectors": ["src/apm_cli/install/dry_run_plan.py"],
+      "decision": "Prospective dry-run install plan and interpreted package selection",
+      "owner": "install/dry_run_plan.py (ProspectiveInstallPlan) delegates selector identity to install/package_selection.py",
+      "selectors": ["src/apm_cli/install/dry_run_plan.py", "src/apm_cli/install/package_selection.py"],
       "guards": ["install-deployment-prospective-dry-run-plan"]
     },
     {

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Git marketplace installs accept versioned plugin directories while rejecting file, symlink, and submodule responses during directory validation. (by @Pybsama, #3156)
+
 ## [0.33.0] - 2026-10-02
 
 ### Added

docs/src/content/docs/consumer/installing-from-marketplaces.md

@@ -112,6 +112,12 @@ identity; bare cross-repository entries retain their normal dependency defaults.
 Invalid URLs and unsafe subdirectory
 paths fail before manifest, lockfile, or deployment writes.
 
+Catalog paths can point to versioned directories generated by `apm pack`, such
+as `plugins/my-plugin-1.2.3`. APM keeps their repository/path coordinates through
+installation and `--dry-run` instead of treating the version suffix as a file
+extension. Directory content and path checks still apply. Git directory probes
+reject files, symbolic links, and submodules.
+
 Use `apm install pkg@catalog#v1.0.1` for a literal Git tag. For a range, declare
 the marketplace dependency in `apm.yml`:
 

packages/apm-guide/.apm/skills/apm-usage/commands.md

@@ -90,6 +90,11 @@ lowercase before constructing the package-registry path.
 
 ### Install validation chain (virtual subdirectory packages)
 
+Marketplace installation supports versioned directories produced by `apm pack`,
+such as `plugins/my-plugin-1.2.3`, including positional `--dry-run` previews.
+Directory content and path validation still apply. Git directory probes reject
+files, symbolic links, and submodules.
+
 `apm install` validates subdirectory packages (`owner/repo/path#ref`) before writing to `apm.yml` using the same credential chain as the actual install. Git-source semver ranges (for example, `owner/repo/path#^1.2.0`) defer raw-ref validation to semver tag resolution; registry-routed dependencies retain registry version validation. See [Authentication > Install validation chain](../authentication/) for the full probe sequence and troubleshooting.
 
 When a default registry is configured, plain shorthand deps (`owner/repo#<ref>`) bypass the GitHub probe. `apm install` requires a version selector before writing to `apm.yml`; deps with no `#<ref>` at all are rejected. Semver selectors (`1.0.0`, `^1.2.3`) use range matching; non-semver selectors (`stable`, `v1.4.2`, any opaque label) are matched exactly against the registry's published versions.

scripts/architecture_linter/checks/install_dry_run_plan.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import ast
 import re
 
 from scripts.architecture_linter.checks.install_deployment_shared import (
@@ -19,6 +20,35 @@
 _OWNER = "src/apm_cli/install/dry_run_plan.py"
 _COMMAND = "src/apm_cli/commands/install.py"
 _RENDERER = "src/apm_cli/install/presentation/dry_run.py"
+_SELECTION_OWNER = "src/apm_cli/install/package_selection.py"
+_RESOLVE = "src/apm_cli/install/phases/resolve.py"
+
+
+def _selection_uses_interpreted_identity(provider: FactsProvider) -> bool:
+    """Install and preview must preserve structured references through one owner."""
+    for path, function_name in (
+        (_OWNER, "ProspectiveInstallPlan.from_apm_package"),
+        (_RESOLVE, "_apply_only_filter"),
+    ):
+        index = provider.tree_index(path)
+        function = index.function(function_name) if index is not None else None
+        if function is None:
+            return False
+        calls = [node for node in ast.walk(function) if isinstance(node, ast.Call)]
+        if not any(
+            isinstance(call.func, ast.Name) and call.func.id == "selected_dependency_identity"
+            for call in calls
+        ):
+            return False
+        if any(
+            isinstance(call.func, ast.Attribute)
+            and isinstance(call.func.value, ast.Name)
+            and call.func.value.id == "DependencyReference"
+            and call.func.attr == "parse"
+            for call in calls
+        ):
+            return False
+    return True
 
 
 def check_prospective_dry_run_plan(provider: FactsProvider) -> tuple[Violation, ...]:
@@ -27,7 +57,8 @@ def check_prospective_dry_run_plan(provider: FactsProvider) -> tuple[Violation,
     owner, owner_fail = _facts_for(provider, _OWNER, rule_id)
     command, command_fail = _facts_for(provider, _COMMAND, rule_id)
     renderer, renderer_fail = _facts_for(provider, _RENDERER, rule_id)
-    failures = list(owner_fail) + list(command_fail) + list(renderer_fail)
+    selection, selection_fail = _facts_for(provider, _SELECTION_OWNER, rule_id)
+    failures = list(owner_fail) + list(command_fail) + list(renderer_fail) + list(selection_fail)
     if failures:
         return tuple(failures)
 
@@ -41,8 +72,20 @@ def check_prospective_dry_run_plan(provider: FactsProvider) -> tuple[Violation,
         message="ProspectiveInstallPlan must remain the sole dry-run preview owner",
         respect_exempt=False,
     )
+    identity_pattern = re.compile(r"^def selected_dependency_identity\(")
+    duplicates += _duplicate_definition_lines(
+        provider,
+        rule_id=rule_id,
+        prefix=_SRC_PREFIX,
+        pattern=identity_pattern,
+        owner=_SELECTION_OWNER,
+        message="Install selector identity must remain owned by package_selection",
+        respect_exempt=False,
+    )
     contract_holds = (
         _count_re(owner, class_pattern) == 1
+        and _count_re(selection, identity_pattern) == 1
+        and _selection_uses_interpreted_identity(provider)
         and _present(owner, "def from_apm_package(")
         and _present(owner, "def with_allowed_lsp_dependencies(")
         and _present(owner, "selected_apm_dependencies=selected_apm_dependencies")
@@ -74,7 +117,7 @@ def check_prospective_dry_run_plan(provider: FactsProvider) -> tuple[Violation,
             rule_id,
             _OWNER,
             "Dry-run dependencies, selection, checks, rendering, and counts must route "
-            "through ProspectiveInstallPlan",
+            "through ProspectiveInstallPlan and the shared package-selection identity owner",
         ),
         *duplicates,
     )

src/apm_cli/deps/github_downloader_validation.py

@@ -238,10 +238,11 @@ def _directory_exists_at_ref(
 
     Uses the default ``Accept: application/vnd.github+json`` so the
     endpoint returns the directory listing for directories (and file
-    metadata for files).  A 200 means the path resolves at the ref,
-    which is what install needs.
+    metadata for files). Only an array of entry objects confirms a
+    directory; file, symlink, and submodule objects do not.
 
-    Returns ``True`` on 200; ``False`` on 404 or any error.  Only
+    Returns ``True`` for a directory listing, including an empty array;
+    ``False`` for non-directory content, 404, or any error. Only
     implemented for github.com / GHE; non-GitHub hosts return ``False``
     and rely on the marker-file probes above.
     """
@@ -287,6 +288,16 @@ def _request(token: str | None, _git_env: dict[str, str]) -> bool:
         )
         raise_for_github_throttle(response, host)
         if response.status_code == 200:
+            try:
+                entries = response.json()
+            except ValueError:
+                log(f"  [x] {path}@{ref} (invalid Contents API JSON)")
+                return False
+            if not isinstance(entries, list) or not all(
+                isinstance(entry, dict) for entry in entries
+            ):
+                log(f"  [!] {path}@{ref} (not a directory listing)")
+                return False
             log(f"  [+] {path}@{ref} (directory)")
             return True
         response.raise_for_status()
@@ -730,7 +741,7 @@ def _path_exists_in_tree_at_ref(
     log: Callable[[str], None],
     winning_attempt: AttemptSpec,
 ) -> bool:
-    """Confirm ``vpath`` exists at ``ref`` via shallow fetch + ``ls-tree``.
+    """Confirm ``vpath`` is a directory at ``ref`` via shallow fetch + ``ls-tree``.
 
     Closes the fail-open hole in ``_ref_exists_via_ls_remote``: knowing
     that the ref exists is not the same as knowing the subdirectory
@@ -748,7 +759,7 @@ def _path_exists_in_tree_at_ref(
 
     Returns:
         True iff the shallow fetch succeeded AND ``ls-tree`` reported
-        at least one entry for ``vpath`` at the resolved ref.
+        a directory entry for ``vpath`` at the resolved ref.
     """
     label, url, env = winning_attempt
 
@@ -804,7 +815,7 @@ def _path_exists_in_tree_at_ref(
 
         try:
             result = subprocess.run(
-                [git_exe, "--git-dir", str(bare), "ls-tree", "FETCH_HEAD", vpath],
+                [git_exe, "--git-dir", str(bare), "ls-tree", "-d", "FETCH_HEAD", vpath],
                 check=True,
                 capture_output=True,
                 text=True,
@@ -816,7 +827,9 @@ def _path_exists_in_tree_at_ref(
             log(f"  [x] ls-tree failed via {label}: {error}")
             return False
 
-        if output and output.strip():
+        # ``-d`` also includes gitlinks (160000 commit), which cannot be
+        # materialized as plugin content without following another repository.
+        if output and output.split(maxsplit=2)[:2] == ["040000", "tree"]:
             log(f"  [+] {vpath}@{ref} present in tree")
             return True
         log(f"  [!] {vpath} not present in tree at {ref}")

src/apm_cli/install/dry_run_plan.py

@@ -6,6 +6,7 @@
 from dataclasses import dataclass, replace
 from typing import Any
 
+from apm_cli.install.package_selection import selected_dependency_identity
 from apm_cli.models.dependency.reference import DependencyReference
 from apm_cli.security.executables import filter_lsp_by_allow_executables
 
@@ -41,7 +42,8 @@ def from_apm_package(
         selected_apm_dependencies = all_apm_dependencies
         if only_packages is not None:
             selected_identities = {
-                DependencyReference.parse(package).get_identity() for package in only_packages
+                selected_dependency_identity(package, all_apm_dependencies)
+                for package in only_packages
             }
             selected_apm_dependencies = tuple(
                 dependency
@@ -60,7 +62,8 @@ def from_apm_package(
             only_packages=tuple(only_packages) if only_packages is not None else None,
             lsp_dependencies=tuple(apm_package.get_lsp_dependencies()),
             updated_apm_identities=frozenset(
-                DependencyReference.parse(package).get_identity() for package in updated_packages
+                selected_dependency_identity(package, all_apm_dependencies)
+                for package in updated_packages
             ),
         )
 

src/apm_cli/install/package_selection.py

@@ -2,16 +2,27 @@
 
 from __future__ import annotations
 
+from collections.abc import Iterable
 from typing import TYPE_CHECKING
 
+from apm_cli.models.dependency.reference import DependencyReference
+
 if TYPE_CHECKING:
     from apm_cli.core.command_logger import _ValidationOutcome
 
 
+def selected_dependency_identity(package: str, dependencies: Iterable[DependencyReference]) -> str:
+    """Resolve a selector using interpreted references before shorthand parsing."""
+    for dependency in dependencies:
+        # Explicit git+path coordinates can contain dotted bundle directories
+        # whose canonical display is not a valid shorthand file reference.
+        if dependency.to_canonical() == package:
+            return dependency.get_identity()
+    return DependencyReference.parse(package).get_identity()
+
+
 def existing_dependency_identities(current_dependencies: list[object]) -> set[str]:
     """Return canonical identities for every parseable manifest dependency."""
-    from apm_cli.models.apm_package import DependencyReference
-
     identities: set[str] = set()
     for entry in current_dependencies:
         try:

src/apm_cli/install/phases/resolve.py

@@ -928,23 +928,22 @@ def _apply_only_filter(ctx: InstallContext) -> None:
     # ------------------------------------------------------------------
     # 7. --only filtering
     # ------------------------------------------------------------------
-    from apm_cli.models.apm_package import DependencyReference
+    from apm_cli.install.package_selection import selected_dependency_identity
 
     # Build identity set from user-supplied package specs.
     # Accepts any input form: git URLs, FQDN, shorthand.
+    tree = ctx.dependency_graph.dependency_tree
+    resolved_refs = [node.dependency_ref for node in tree.nodes.values()]
     only_identities: builtins.set = builtins.set()
     for p in ctx.only_packages:
         try:
-            ref = DependencyReference.parse(p)
-            only_identities.add(ref.get_identity())
+            only_identities.add(selected_dependency_identity(p, resolved_refs))
         except Exception:
             only_identities.add(p)
 
     # Expand the set to include transitive descendants of the
     # requested packages so their MCP servers, primitives, etc.
     # are correctly installed and written to the lockfile.
-    tree = ctx.dependency_graph.dependency_tree
-
     def _collect_descendants(node: object, visited: builtins.set | None = None) -> None:
         """Walk the tree and add every child identity (cycle-safe)."""
         if visited is None:

src/apm_cli/marketplace/resolver.py

@@ -35,6 +35,7 @@
 from ..deps.transport_selection import initial_transport_scheme
 from ..models.dependency.host_virtual import dependency_repository_owner
 from ..models.dependency.reference import DependencyReference
+from ..models.validation import InvalidVirtualPackageExtensionError
 from ..utils.github_host import (
     build_ado_ssh_url,
     build_ssh_url,
@@ -975,15 +976,20 @@ def _emit_warning(msg: str) -> None:
             plugin_root=manifest.plugin_root,
         )
 
-    if (
-        dep_ref is None
-        and _source_needs_explicit_git_path(source)
-        and _is_in_marketplace_source(plugin, source)
-    ):
+    if dep_ref is None and _is_in_marketplace_source(plugin, source):
         in_repo_path, path_ref = _extract_in_repo_path_and_ref(
             plugin, plugin_root=manifest.plugin_root
         )
-        if in_repo_path:
+        needs_explicit_path = _source_needs_explicit_git_path(source)
+        if in_repo_path and not needs_explicit_path:
+            try:
+                DependencyReference.parse(canonical)
+            except InvalidVirtualPackageExtensionError:
+                # The catalog already identifies the repository and path. A
+                # dotted bundle directory need not fit shorthand's file-name
+                # heuristic; the normal downloader still validates its content.
+                needs_explicit_path = True
+        if in_repo_path and needs_explicit_path:
             # Fall back to the marketplace's registered ref when the plugin
             # source itself declares no ref and no version_spec overrides it.
             # "main" / "HEAD" are excluded because they represent the default

tests/integration/test_architecture_owner_rule_mutations.py

@@ -55,6 +55,33 @@
 OWNERS_DIR = ROOT / ".apm/architecture/owners"
 
 
+@pytest.mark.parametrize(
+    ("path", "old", "new"),
+    [
+        (
+            "src/apm_cli/install/phases/resolve.py",
+            "selected_dependency_identity(p, resolved_refs)",
+            "DependencyReference.parse(p).get_identity()",
+        ),
+        (
+            "src/apm_cli/install/dry_run_plan.py",
+            "selected_dependency_identity(package, all_apm_dependencies)",
+            "DependencyReference.parse(package).get_identity()",
+        ),
+    ],
+)
+def test_package_selector_identity_cannot_bypass_interpreted_references(
+    path: str, old: str, new: str
+) -> None:
+    """Install and preview cannot revert to lossy shorthand reparsing."""
+    rule_id = "install-deployment-prospective-dry-run-plan"
+    source = (ROOT / path).read_text(encoding="utf-8")
+    assert old in source
+    report = run_selected_rules(ROOT, (rule_id,), source_overrides={path: source.replace(old, new)})
+    assert report.failures == ()
+    assert any(violation.rule_id == rule_id for violation in report.violations)
+
+
 @dataclass(frozen=True)
 class MutationCase:
     """One guard's minimal source mutation and the rule that must catch it.

tests/integration/test_integrators_hooks_execution.py

@@ -330,6 +330,7 @@ def test_github_200_returns_true(self) -> None:
 
         mock_resp = MagicMock()
         mock_resp.status_code = 200
+        mock_resp.json.return_value = []
         with patch.object(downloader, "_resilient_get", return_value=mock_resp):
             result = _directory_exists_at_ref(downloader, dep, "skills/foo", "main", lambda m: None)
 
@@ -372,6 +373,7 @@ def test_no_token_still_makes_request(self) -> None:
 
         mock_resp = MagicMock()
         mock_resp.status_code = 200
+        mock_resp.json.return_value = []
         with patch.object(downloader, "_resilient_get", return_value=mock_resp) as mock_get:
             result = _directory_exists_at_ref(downloader, dep, "path", "ref", lambda m: None)
 

tests/integration/test_integrators_hooks_phase3c.py

@@ -330,6 +330,7 @@ def test_github_200_returns_true(self) -> None:
 
         mock_resp = MagicMock()
         mock_resp.status_code = 200
+        mock_resp.json.return_value = []
         with patch.object(downloader, "_resilient_get", return_value=mock_resp):
             result = _directory_exists_at_ref(downloader, dep, "skills/foo", "main", lambda m: None)
 
@@ -372,6 +373,7 @@ def test_no_token_still_makes_request(self) -> None:
 
         mock_resp = MagicMock()
         mock_resp.status_code = 200
+        mock_resp.json.return_value = []
         with patch.object(downloader, "_resilient_get", return_value=mock_resp) as mock_get:
             result = _directory_exists_at_ref(downloader, dep, "path", "ref", lambda m: None)