fix(marketplace): propagate marketplace --ref to string sources; fix GitLab slash-ref proxy filenames

CHANGELOG.md

@@ -9,14 +9,32 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+- Deterministic Artifactory boundary probe: at install time, `_resolve_artifactory_boundary` HEAD-probes the candidate archive URLs and rebuilds the dependency reference at the proxy-verified split. Covers both explicit-FQDN (`host/artifactory/key/owner/repo/...`) and bare-shorthand-under-proxy deps. Distinguishes "missing repo" from auth (401/403) errors; uses `allow_redirects=False` so the bearer token can't leak cross-host. Mirrors the native GitLab probing pattern but without a separate metadata API. (#1472)
+- `ref:` on git-source dependencies now accepts semver ranges (`^1.2.0`, `~1.4`, `>=2.0 <3`, `1.5.x`). `apm install` runs `git ls-remote`, picks the highest tag matching the range, and pins the resolved tag, commit SHA, version, and original constraint in `apm.lock.yaml`. Subsequent installs replay the lockfile without network; use `apm install --update` to re-resolve against current remote tags. Two tag patterns are tried in order (`v{version}`, `{name}--v{version}`) with a bare `{version}` fallback. (closes #1488)
+- `apm deps why <package>` explains why a transitive dependency is installed by walking the lockfile's `resolved_by` chain back to the user's direct declaration in `apm.yml`. Supports `--global` for user-scope lockfiles and `--json` for scriptable output (JSON to stdout, all logs to stderr; analogue of `npm why` / `yarn why`). Exits `0` on success, `1` when the package isn't installed or the query is ambiguous, `2` when no lockfile exists. (#1490)
 - Org-wide policy discovery now cascades through candidate repo names
   (`.github`, then `.apm`, then `_apm`) and speaks the Azure DevOps Items
   API, so Azure DevOps organizations -- which forbid repo names that begin
   or end with `.` -- can host an APM governance policy repo for the first
   time. (by @sergio-sisternes-epam; closes #1813) (#1830)
 
+### Changed
+
+- **BREAKING:** `apm install` now exits `1` whenever the diagnostic summary reports `Installation failed with N error(s)`. Previously the command exited `0` even after reporting errors, so CI could not detect failure via exit code. `--force` continues to bypass only the security scan's critical-finding block; it does **not** suppress general install errors (matches `npm` / `pip` / `cargo`). Callers that asserted `exit_code == 0` while errors were reported must update.
+- Parse-time Artifactory boundary detection no longer uses directory-marker heuristics (`skills/`, `prompts/`, `agents/`, `collections/`, `instructions/`). The install-time resolver is authoritative for the (owner, repo, virtual_path) split. Parse-time defaults differ by mode:
+  - **Explicit FQDN** (`host/artifactory/key/owner/repo/...`): `parse_artifactory_path` stays intentionally shallow -- `owner` = first segment after the prefix, `repo` = next segment, remainder = `virtual_path`.
+  - **Bare shorthand under `PROXY_REGISTRY_ONLY`**: `_bare_shorthand_repo_segment_count` defaults to all-as-repo with a structural file-extension rule on the last segment (a path ending in `.prompt.md`/`.instructions.md`/`.chatmode.md`/`.agent.md` is by shape a virtual file; everything before it is the repo).
+
+  Both modes converge at install time: `_resolve_artifactory_boundary` HEAD-probes candidate splits and rebuilds the dependency reference at the proxy-verified split. The `_VIRTUAL_PATH_ROOT_SEGMENTS`, `_ARTIFACTORY_VIRTUAL_MARKERS`, and `_ARTIFACTORY_VIRTUAL_FILE_EXTENSIONS` constants are removed. (#1472)
+
 ### Fixed
 
+- `apm install` through a registry proxy now supports nested-group repos (3+ path segments, e.g. `group/subgroup/project`). Previously every trailing segment past `owner/repo` was treated as an in-repo virtual sub-path, so the downloader requested the wrong archive URL and the install failed with HTTP 404 from the proxy. Behavior is gated on `PROXY_REGISTRY_ONLY` so direct (non-proxy) installs keep the legacy two-segment shape. Affects `parse_artifactory_path`, `build_artifactory_archive_url`, `_detect_virtual_package`, the shorthand resolvers in `DependencyReference`, and `ArtifactoryOrchestrator._split_owner_repo` / `download_subdirectory`. (#1472)
+- URL-form Artifactory deps no longer round-trip with the `artifactory/<key>` prefix folded into `repo_url`. The duplicated prefix caused the downloader to construct double-prefixed archive URLs (`/artifactory/key/artifactory/key/owner/repo/...`) and 404. `_validate_url_repo_path` now strips the Artifactory VCS prefix before returning the bare `owner/repo` slug; the prefix is still recovered separately via `_extract_artifactory_prefix`. (#1472)
+- `apm install --update` now re-resolves direct git-source semver dependencies. Previously, when the dependency's install path already existed on disk, the BFS resolver short-circuited and `--update` was a silent no-op for git-semver refs; the lockfile kept the previously-resolved tag.
+- `policy.dependencies.require_pinned_constraint: true` no longer misclassifies the npm- and cargo-style explicit-equality form `=1.2.3` as `BARE_BRANCH`. Both `1.2.3` and `=1.2.3` are now recognized as pinned constraints; the pip-style `==1.2.3` form is still rejected (not part of node-semver). Follow-up to #1494 / #1505.
+- `apm install plugin@marketplace` now correctly fetches from the marketplace's registered `--ref` branch instead of silently falling back to the repository's default branch. Root cause: `resolve_marketplace_plugin` did not propagate `source.ref` to downstream resolution calls. Covers both GitHub-family hosts (ref appended to canonical as `#ref`) and GitLab-hosted marketplaces (ref injected into `DependencyReference`). Guards prevent double-injection when a plugin's own dict source already carries an explicit `ref`, and skip `main`/`HEAD` as implicit defaults. (mirrors [microsoft/apm#1824](https://github.com/microsoft/apm/pull/1824))
+- GitLab archive URLs with slash-containing branch names (e.g. `feat/my-feature`) now produce correctly-formed Artifactory-proxyable filenames. The slash is preserved in the path segment (as the GitLab archive API expects) but replaced with `-` in the archive filename, matching GitLab's own naming convention. Previously, `PROXY_REGISTRY_ONLY=1` installs from such branches returned HTTP 404 from the proxy because the generated filename contained a literal slash. This is the proxy scenario not covered by [microsoft/apm#1824](https://github.com/microsoft/apm/pull/1824).
 - `apm install <pkg>@<marketplace>` now preserves GitLab and other
   non-GitHub hosts from url-type marketplace plugin sources, so auth
   resolution no longer falls back to `github.com` for those installs.

src/apm_cli/install/artifactory_resolver.py

@@ -93,6 +93,7 @@ def _candidate_archive_status(
     """
     urls = build_artifactory_archive_url(host, prefix, owner, repo, ref, scheme=scheme)
     saw_auth = False
+    saw_non_auth_4xx = False
     saw_any_response = False
     last_exc: BaseException | None = None
     for url in urls:
@@ -111,9 +112,14 @@ def _candidate_archive_status(
             return _CandidateStatus.EXISTS, None
         if r.status_code in (401, 403):
             saw_auth = True
+        else:
+            saw_non_auth_4xx = True
     if not saw_any_response:
         return _CandidateStatus.INCONCLUSIVE, last_exc
-    if saw_auth:
+    # AUTH only when *every* response was 401/403 -- no non-auth 4xx seen.
+    # A mixed 401+404 means at least one URL shape was definitively absent;
+    # report MISSING so the user-facing error stays accurate (per module spec).
+    if saw_auth and not saw_non_auth_4xx:
         return _CandidateStatus.AUTH, None
     return _CandidateStatus.MISSING, None
 

src/apm_cli/utils/github_host.py

@@ -775,7 +775,10 @@ def build_artifactory_archive_url(
         # GitHub-style: /archive/refs/heads/{ref}.zip
         f"{base}/archive/refs/heads/{ref}.zip",
         # GitLab-style: /-/archive/{ref}/{basename}-{ref}.zip
-        f"{base}/-/archive/{ref}/{repo_basename}-{ref}.zip",
+        # GitLab keeps the raw ref (including slashes) as a path segment but replaces
+        # slashes with dashes in the archive *filename*.  E.g. branch
+        # ``feat/my-feature`` -> ``.../-/archive/feat/my-feature/repo-feat-my-feature.zip``.
+        f"{base}/-/archive/{ref}/{repo_basename}-{ref.replace('/', '-')}.zip",
         # GitHub-style tags fallback
         f"{base}/archive/refs/tags/{ref}.zip",
         # codeload.github.com-style: /zip/refs/heads/{ref}

tests/unit/marketplace/test_marketplace_resolver.py

@@ -1507,102 +1507,145 @@ def test_fqdn_shorthand_without_git_path_misclassifies(self, monkeypatch):
         assert good.virtual_path == "agents/reverse-architect"
 
 
-class TestMarketplaceRefPropagation:
-    """Registered --ref must propagate to relative string plugin sources (#1811)."""
+class TestMarketplaceRegisteredRefPropagation:
+    """Registered marketplace ``--ref`` must propagate to plugin canonicals.
 
-    @staticmethod
-    def _manifest_with_plugin(plugin: MarketplacePlugin) -> MarketplaceManifest:
-        return MarketplaceManifest(name="mkt", plugins=(plugin,), plugin_root="")
+    When a marketplace is added with ``apm marketplace add --ref feat/x owner/repo``,
+    subsequent ``apm install plugin@marketplace`` must download the plugin from
+    ``feat/x`` -- not from the repo's default branch.  This applies to both:
 
-    @patch("apm_cli.marketplace.resolver.fetch_or_cache")
-    @patch("apm_cli.marketplace.resolver.get_marketplace_by_name")
-    def test_github_relative_source_propagates_registered_ref(self, mock_get, mock_fetch):
-        """A GitHub marketplace registered with --ref feat/xxx appends #feat/xxx."""
-        source = MarketplaceSource(
-            name="mkt",
+    - GitHub-family marketplaces (no ``dependency_reference``; ref embedded in canonical)
+    - GitLab / non-GitHub marketplaces (ref embedded in ``dependency_reference``)
+    """
+
+    @staticmethod
+    def _github_source(ref: str = "main") -> MarketplaceSource:
+        return MarketplaceSource(
+            name="my-marketplace",
             owner="acme",
-            repo="catalog",
+            repo="plugins",
             host="github.com",
-            ref="feat/my-feature",
+            ref=ref,
         )
+
+    @staticmethod
+    def _gitlab_source(ref: str = "main") -> MarketplaceSource:
+        return MarketplaceSource(
+            name="my-marketplace",
+            owner="acme",
+            repo="plugins",
+            host="gitlab.com",
+            ref=ref,
+        )
+
+    @staticmethod
+    def _manifest(plugin: MarketplacePlugin) -> MarketplaceManifest:
+        return MarketplaceManifest(name="my-marketplace", plugins=(plugin,), plugin_root="")
+
+    # -- GitHub-family: ref embedded in canonical ---------------------------------
+
+    @patch("apm_cli.marketplace.resolver.fetch_or_cache")
+    @patch("apm_cli.marketplace.resolver.get_marketplace_by_name")
+    def test_github_string_source_registered_ref_appended(self, mock_get, mock_fetch):
+        """Non-main marketplace ref must appear as ``#ref`` suffix in the canonical."""
         plugin = MarketplacePlugin(name="my-plugin", source="./plugins/my-plugin")
-        mock_get.return_value = source
-        mock_fetch.return_value = self._manifest_with_plugin(plugin)
+        mock_get.return_value = self._github_source(ref="feat/my-feature")
+        mock_fetch.return_value = self._manifest(plugin)
+
+        result = resolve_marketplace_plugin("my-plugin", "my-marketplace")
 
-        result = resolve_marketplace_plugin("my-plugin", "mkt")
-        assert result.canonical == "acme/catalog/plugins/my-plugin#feat/my-feature"
+        assert result.canonical == "acme/plugins/plugins/my-plugin#feat/my-feature"
         assert result.dependency_reference is None
 
     @patch("apm_cli.marketplace.resolver.fetch_or_cache")
     @patch("apm_cli.marketplace.resolver.get_marketplace_by_name")
-    def test_github_relative_source_main_ref_not_appended(self, mock_get, mock_fetch):
-        """ref='main' is the default -- do not append #main to the canonical."""
-        source = MarketplaceSource(
-            name="mkt",
-            owner="acme",
-            repo="catalog",
-            host="github.com",
-            ref="main",
-        )
+    def test_github_string_source_main_ref_not_appended(self, mock_get, mock_fetch):
+        """Default ``main`` ref must NOT be embedded (implicit; avoids cluttering canonicals)."""
         plugin = MarketplacePlugin(name="my-plugin", source="./plugins/my-plugin")
-        mock_get.return_value = source
-        mock_fetch.return_value = self._manifest_with_plugin(plugin)
+        mock_get.return_value = self._github_source(ref="main")
+        mock_fetch.return_value = self._manifest(plugin)
 
-        result = resolve_marketplace_plugin("my-plugin", "mkt")
-        assert result.canonical == "acme/catalog/plugins/my-plugin"
+        result = resolve_marketplace_plugin("my-plugin", "my-marketplace")
+
+        assert "#" not in result.canonical
 
     @patch("apm_cli.marketplace.resolver.fetch_or_cache")
     @patch("apm_cli.marketplace.resolver.get_marketplace_by_name")
-    def test_version_spec_overrides_registered_ref(self, mock_get, mock_fetch):
-        """Explicit version_spec takes precedence over registered ref."""
-        source = MarketplaceSource(
-            name="mkt",
-            owner="acme",
-            repo="catalog",
-            host="github.com",
-            ref="feat/my-feature",
-        )
+    def test_github_string_source_head_ref_not_appended(self, mock_get, mock_fetch):
+        """``HEAD`` ref, like ``main``, must not be embedded."""
         plugin = MarketplacePlugin(name="my-plugin", source="./plugins/my-plugin")
-        mock_get.return_value = source
-        mock_fetch.return_value = self._manifest_with_plugin(plugin)
+        mock_get.return_value = self._github_source(ref="HEAD")
+        mock_fetch.return_value = self._manifest(plugin)
 
-        result = resolve_marketplace_plugin("my-plugin", "mkt", version_spec="v2.0.0")
-        assert result.canonical == "acme/catalog/plugins/my-plugin#v2.0.0"
+        result = resolve_marketplace_plugin("my-plugin", "my-marketplace")
+
+        assert "#" not in result.canonical
 
     @patch("apm_cli.marketplace.resolver.fetch_or_cache")
     @patch("apm_cli.marketplace.resolver.get_marketplace_by_name")
-    def test_gitlab_relative_source_propagates_registered_ref(self, mock_get, mock_fetch):
-        """A GitLab marketplace registered with --ref feat/xxx sets ref in dep_ref."""
-        source = MarketplaceSource(
-            name="mkt",
-            owner="acme",
-            repo="catalog",
-            host="gitlab.com",
-            ref="feat/my-feature",
+    def test_github_string_source_version_spec_overrides_registered_ref(
+        self, mock_get, mock_fetch
+    ):
+        """Explicit ``version_spec`` (``apm install plugin@mkt#v2``) must win over source.ref."""
+        plugin = MarketplacePlugin(name="my-plugin", source="./plugins/my-plugin")
+        mock_get.return_value = self._github_source(ref="feat/my-feature")
+        mock_fetch.return_value = self._manifest(plugin)
+
+        result = resolve_marketplace_plugin(
+            "my-plugin", "my-marketplace", version_spec="v2.0.0"
         )
-        plugin = MarketplacePlugin(name="my-plugin", source="registry/my-plugin")
-        mock_get.return_value = source
-        mock_fetch.return_value = self._manifest_with_plugin(plugin)
 
-        result = resolve_marketplace_plugin("my-plugin", "mkt")
-        assert result.dependency_reference is not None
-        assert result.dependency_reference.reference == "feat/my-feature"
+        assert result.canonical.endswith("#v2.0.0")
+        assert "feat/my-feature" not in result.canonical
 
     @patch("apm_cli.marketplace.resolver.fetch_or_cache")
     @patch("apm_cli.marketplace.resolver.get_marketplace_by_name")
-    def test_gitlab_relative_source_main_ref_not_propagated(self, mock_get, mock_fetch):
-        """ref='main' should not be propagated to the GitLab dep_ref."""
-        source = MarketplaceSource(
-            name="mkt",
-            owner="acme",
-            repo="catalog",
-            host="gitlab.com",
-            ref="main",
+    def test_github_dict_source_registered_ref_not_propagated(self, mock_get, mock_fetch):
+        """Dict sources with an explicit ``ref`` must not be overwritten by source.ref."""
+        plugin = MarketplacePlugin(
+            name="dict-plugin",
+            source={"type": "github", "repo": "acme/plugins", "path": "p", "ref": "v1.0.0"},
+        )
+        mock_get.return_value = self._github_source(ref="feat/my-feature")
+        mock_fetch.return_value = self._manifest(plugin)
+
+        result = resolve_marketplace_plugin("dict-plugin", "my-marketplace")
+
+        assert result.canonical.endswith("#v1.0.0"), (
+            "Explicit plugin ref must survive; marketplace ref must not overwrite it"
         )
+
+    # -- GitLab: ref embedded in dependency_reference ----------------------------
+
+    @patch("apm_cli.marketplace.resolver.fetch_or_cache")
+    @patch("apm_cli.marketplace.resolver.get_marketplace_by_name")
+    def test_gitlab_string_source_registered_ref_propagated(self, mock_get, mock_fetch):
+        """GitLab monorepo: marketplace ref must reach dep_ref when path_ref is None."""
         plugin = MarketplacePlugin(name="my-plugin", source="registry/my-plugin")
-        mock_get.return_value = source
-        mock_fetch.return_value = self._manifest_with_plugin(plugin)
+        mock_get.return_value = self._gitlab_source(ref="feat/my-feature")
+        mock_fetch.return_value = self._manifest(plugin)
+
+        result = resolve_marketplace_plugin("my-plugin", "my-marketplace")
 
-        result = resolve_marketplace_plugin("my-plugin", "mkt")
         assert result.dependency_reference is not None
-        assert result.dependency_reference.reference is None
+        dep = result.dependency_reference
+        assert dep.reference == "feat/my-feature", (
+            "Registered marketplace ref must be threaded into the DependencyReference"
+        )
+        assert dep.virtual_path == "registry/my-plugin"
+
+    @patch("apm_cli.marketplace.resolver.fetch_or_cache")
+    @patch("apm_cli.marketplace.resolver.get_marketplace_by_name")
+    def test_gitlab_string_source_main_ref_not_propagated(self, mock_get, mock_fetch):
+        """Default ``main`` ref on a GitLab marketplace must not produce a dep_ref ref field."""
+        plugin = MarketplacePlugin(name="my-plugin", source="registry/my-plugin")
+        mock_get.return_value = self._gitlab_source(ref="main")
+        mock_fetch.return_value = self._manifest(plugin)
+
+        result = resolve_marketplace_plugin("my-plugin", "my-marketplace")
+
+        dep = result.dependency_reference
+        assert dep is not None
+        assert dep.reference is None, (
+            "Default 'main' ref must not be embedded in DependencyReference"
+        )