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

[Ron Izraeli (chkp-roniz)]

Summary

• Ref propagation — 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. resolve_marketplace_plugin was not propagating 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 dict source already declares its own ref, and skip main/HEAD as implicit defaults. Mirrors #1824.
• GitLab slash-ref proxy filenames — 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 requires) 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 because the generated filename contained a literal slash. This is the proxy scenario that #1824 did not cover.

What #1824 was missing

#1824 fixed ref propagation for the resolution path but left a gap in the Artifactory proxy path: build_artifactory_archive_url emitted GitLab archive filenames with a literal / when the branch contained a slash. Artifactory rejects such filenames with HTTP 404 under PROXY_REGISTRY_ONLY=1. This PR completes the fix.

Test plan

• 7 new regression tests in TestMarketplaceRegisteredRefPropagation — GitHub-family and GitLab ref propagation, main/HEAD exclusion, version_spec override precedence, dict-source non-overwrite
• 2 new tests in TestBuildArtifactoryArchiveUrl — slash-ref filename normalisation and single-component ref stability
• 9/9 new tests pass; 0 pre-existing test failures introduced (4 pre-existing failures on main are unrelated)
• CHANGELOG updated in [Unreleased] > Fixed

Follow-ups (not blockers, carried from #1824 review panel)

• Defense-in-depth _validate_ref() call on injected refs
• Additional HEAD-exclusion regression test (covered here in test_github_string_source_head_ref_not_appended)

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR improves dependency resolution in two main areas: (1) propagating a marketplace-registered --ref through to plugin resolution so installs don’t silently fall back to the default branch, and (2) making Artifactory/GitLab archive URL handling more robust (including slash-containing refs and nested subgroup repo paths) by introducing an install-time Artifactory boundary probe and updating parsing/orchestration accordingly.

Changes:

• Propagate marketplace source.ref into downstream plugin canonical/dependency-reference resolution (GitHub-family #ref suffix; GitLab via DependencyReference.reference).
• Add deterministic Artifactory boundary probing (HEAD archive URLs) and wire it into install-time dependency resolution, including support for nested GitLab subgroup repos behind a proxy.
• Normalize GitLab archive filenames for slash-containing refs and adjust Artifactory URL building and parsing to avoid malformed proxy paths.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
src/apm_cli/marketplace/resolver.py	Threads marketplace --ref into plugin resolution and adds guard rails around ref injection/override.
src/apm_cli/utils/github_host.py	Adds Artifactory boundary candidate enumeration, refines Artifactory parsing, and fixes GitLab archive filename construction (basename + slash-ref normalization).
src/apm_cli/install/artifactory_resolver.py	Introduces install-time Artifactory boundary resolver that probes archive URLs to confirm the correct owner/repo/virtual split.
src/apm_cli/install/package_resolution.py	Plumbs the Artifactory boundary resolver into dependency reference resolution and persistence decisions.
src/apm_cli/models/dependency/reference.py	Adds Artifactory probe ref constructor and adjusts shorthand parsing/URL validation for proxy/nested paths.
src/apm_cli/deps/artifactory_orchestrator.py	Updates owner/repo splitting to preserve nested subgroup repo slugs.
src/apm_cli/commands/install.py	Wires Artifactory boundary resolution into apm install and updates persistence condition naming.
tests/unit/marketplace/test_marketplace_resolver.py	Adds regression tests for marketplace registered-ref propagation (GitHub-family + GitLab).
tests/unit/test_artifactory_support.py	Expands Artifactory parsing/probing/URL-construction tests, including nested subgroup and slash-ref behaviors.
tests/unit/install/test_artifactory_resolver.py	Adds focused regression traps for redirect/token-audience/error-classification invariants in the Artifactory resolver.
tests/unit/deps/test_artifactory_orchestrator.py	Adds tests ensuring subgroup folding in _split_owner_repo is preserved.
docs/src/content/docs/enterprise/registry-proxy.md	Documents nested-group repo behavior behind the registry proxy and the deterministic boundary probe.
CHANGELOG.md	Adds Unreleased notes for the new Artifactory boundary probing behavior and the marketplace/GitLab fixes.

[Daniel Meppiel (danielmeppiel)]

APM Review Panel: ship_with_followups

Community bug-fix PR from Check Point (chkp-roniz) correctly fixes marketplace --ref propagation and GitLab slash-ref proxy filenames; no security regression; ship after CHANGELOG fold pass.

cc Ron Izraeli (@chkp-roniz) Daniel Meppiel (@danielmeppiel) Sergio Sisternes (@sergio-sisternes-epam) -- a fresh advisory pass is ready for your review.

All eight panelists active; zero blocking findings remain after accounting for the concurrent shepherd fold pass that will restore CHANGELOG entries and fix stale vocabulary. Auth-expert and supply-chain-security-expert both cleared the behavioral changes with no findings above nit. Python-architect's recommended extraction of ref-propagation helpers is sound architectural hygiene but does not block a bug-fix PR -- it belongs in a follow-up refactor issue.

The test-coverage-expert flagged a missing integration-tier test for the marketplace --ref propagation flow. Per contract, a missing-test finding on a user-facing promise surface ranks above opinion-only recommended findings. However, the 7 unit tests are described as strong mutation-break traps covering the resolver logic in isolation, and the bug being fixed is a straightforward data-propagation gap (ref not forwarded to downstream resolution). The integration gap is real but does not block ship for a community bug-fix that already has unit-level proof of correctness. File as a follow-up issue.

No panelist dissent exists -- all findings are nit or recommended tier, and no two panelists disagreed on severity classification of any finding.

Aligned with: Developer experience (fixes silent wrong-branch resolution; error messages now correctly distinguish AUTH vs MISSING); Reliability (slash-containing branch names no longer produce malformed proxy filenames; regression traps added); Portability (extends correct behavior to GitLab+Artifactory proxy topology); Security (supply-chain and auth experts confirmed no regression).

Growth signal. This PR is a textbook community-contributor success story: chkp-roniz (Check Point) filed #1472, iterated through panel review on #1824, and now completes the gap. In the next release narrative, call out the contributor and frame as 'APM now works correctly behind enterprise artifact proxies with slash-containing branch names' -- high credibility signal for the GitLab/Artifactory enterprise segment.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	1	2	Correct bug fixes with good test coverage; marketplace resolver function continues growing and would benefit from extraction of ref-propagation logic.
Supply Chain Security	0	0	2	No security regression: slash-to-dash is filename-only, ref source is local user config, AUTH/MISSING logic is correct for single-URL case.
Auth Expert	0	0	0	Auth classification change is correct: single-host 401-only still returns AUTH; allow_redirects=False and token-audience routing preserved.
Test Coverage Expert	0	1	1	Unit tests are strong mutation-break traps for all three changes; integration-tier gap for marketplace --ref propagation flow is the only substantive finding.
Doc Writer	0	0	1	CHANGELOG fix in progress (shepherd fold pass); registry-proxy.md additions are clean.
DevX UX Expert	0	0	2	Solid bug-fix UX: error messages distinguish AUTH vs MISSING clearly; ref propagation guards are sensible; no user-facing regressions.
CLI Logging Expert	0	0	0	No CLI output regressions; error reclassification is correct; marketplace debug logs follow conventions.
OSS Growth Hacker	0	0	2	Community fix unblocks GitLab enterprise segment; consider amplifying in release notes and contributor shout-out.
Performance Expert	0	0	2	No perf regression in artifactory_resolver (same HTTP call count). ADO REST fast-path removal adds ~200-500ms/source for ADO marketplace fetches -- acceptable simplification.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 4 follow-ups

1. [Test Coverage Expert] Add integration test for apm install plugin@marketplace with non-default --ref -- Missing integration-tier coverage on a user-facing promise surface; unit tests prove logic but not the full install flow wiring.
2. [Python Architect] Extract ref-propagation helpers from resolve_marketplace_plugin (~200-line function) -- Five sequential ref-propagation blocks accumulating; extraction improves testability and readability without changing behavior.
3. [DevX UX Expert] Add logger.debug when marketplace ref guard fires on non-main default branch -- Silent ref-drop is confusing for users whose default branch is not named main; debug log aids troubleshooting.
4. [Supply Chain Security] URL-encode ref values before interpolation into archive URLs (pre-existing tech debt) -- Not introduced by this PR but noted; special characters in ref names could produce malformed URLs.

Recommendation

Ship after the shepherd fold pass restores CHANGELOG entries. Two follow-up issues: integration test for marketplace --ref flow, and resolver refactor to extract ref-propagation helpers.

---

Full per-persona findings

Python Architect

• [recommended] resolve_marketplace_plugin is accumulating ref-logic blocks that should be extracted at src/apm_cli/marketplace/resolver.py:842
  The function is ~200 lines with five sequential ref-propagation/override blocks. Extracting _build_url_source_dep_ref and _resolve_version_spec_canonical as standalone helpers would improve testability and readability without changing behavior.
  Suggested: Extract _build_url_source_dep_ref(plugin, source, version_spec) and _resolve_version_spec_canonical(canonical, source, version_spec, plugin_name, marketplace_name, auth_resolver) as standalone helpers.

• [nit] Underscore-prefixed locals inside url-source block are unconventional for non-throwaway values at src/apm_cli/marketplace/resolver.py:846
  Python convention reserves leading underscores for unused/private names. _url, _ref, _effective_ref, _entry are all actively used.

• [nit] The main/HEAD exclusion set is duplicated across two blocks; use a module-level frozenset constant at src/apm_cli/marketplace/resolver.py:914
  Suggested: _DEFAULT_BRANCH_REFS = frozenset({"main", "HEAD"}) then source.ref not in _DEFAULT_BRANCH_REFS

Supply Chain Security Expert

• [nit] Ref value is URL-interpolated without encoding at src/apm_cli/utils/github_host.py:781
  Pre-existing; not introduced by this PR. A ref containing #, ?, or % would corrupt the URL. Git rejects most problematic characters in ref names so practical risk is low. Consider quote(ref, safe='/') for path segment in a follow-up.

• [nit] source.ref guard pattern differs slightly between two sites at src/apm_cli/marketplace/resolver.py:916
  Minor inconsistency in defensive pattern -- both are safe in practice.

Auth Expert

No findings.

Test Coverage Expert

• [recommended] Marketplace --ref propagation has no integration-tier test exercising the full install flow
  7 unit tests mock fetch_or_cache and get_marketplace_by_name, proving the resolver logic in isolation. No integration test exercises apm install plugin@marketplace where marketplace was added with non-default --ref. The unit tests are solid and mutation-breaking, so this is recommended not blocking.
  Proof (missing): tests/integration/marketplace/test_ref_propagation_e2e.py::test_install_from_marketplace_with_non_default_ref_downloads_correct_branch -- proves: apm install plugin@marketplace respects the --ref registered on the marketplace source

• [nit] saw_non_auth_4xx regression trap is well-crafted and mutation-breaking at tests/unit/install/test_artifactory_resolver.py
  Proof (passed): tests/unit/install/test_artifactory_resolver.py::TestErrorDiscrimination::test_mixed_401_and_404_demotes_to_unresolved -- proves: Mixed 401+404 responses are classified as MISSING not AUTH, preventing user-facing misdiagnosis

Doc Writer

CHANGELOG [Unreleased] restoration in progress via shepherd fold pass. registry-proxy.md additions are accurate and well-structured with no issues.

• [nit] registry-proxy.md new sections are accurate and well-structured; no issues

DevX UX Expert

• [nit] Error message uses RST backticks in CLI output at src/apm_cli/install/artifactory_resolver.py:48
_ARTIFACTORY_BOUNDARY_UNRESOLVED and _ARTIFACTORY_BOUNDARY_AUTH constants use double-backtick RST notation. Use single backticks or no backticks for CLI-facing error strings.

• [nit] Silent ref-drop when default branch is not 'main' at src/apm_cli/marketplace/resolver.py:836
  The guard silently drops the ref when the user explicitly pinned --ref main but the repo's default has a different name. Consider a logger.debug when the guard fires for easier troubleshooting.

CLI Logging Expert

No findings.

OSS Growth Hacker

• [nit] CHANGELOG entries deserve a 'Enterprise/GitLab' section header or callout in release narrative
  The two CHANGELOG bullets are excellent -- they name the symptom, affected persona, and lineage. When cutting the next release, these deserve a dedicated callout.

• [nit] Add troubleshooting FAQ entry: 'My marketplace plugin installs from the wrong branch' pointing to verbose debug log
  The new debug log 'Propagated marketplace ref ... to string source canonical' is exactly the breadcrumb an enterprise user would grep for.

Performance Expert

• [nit] No extra HTTP calls from saw_non_auth_4xx guard -- classification-only change
  The _candidate_archive_status change adds zero additional HTTP requests. The loop still iterates the same URL set and exits early on the first 2xx/3xx. The new flag is a pure classification refinement post-loop.

• [nit] ADO REST fast-path removal trades ~200-500ms/source for code simplification -- acceptable
  The deleted _fetch_ado_rest path was a single HTTP GET. The replacement git-subprocess path adds process-spawn overhead. Acceptable trade-off as marketplace fetches are not in the install hot loop.

_{This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.}

[Daniel Meppiel (danielmeppiel)]

APM Review Panel: ship_now

Ship: bounded community bug fix with full regression coverage, no blocking findings, all triage reservations resolved.

cc Ron Izraeli (@chkp-roniz) Daniel Meppiel (@danielmeppiel) Sergio Sisternes (@sergio-sisternes-epam) -- a fresh advisory pass is ready for your review.

Nine panelists reviewed; zero blocking findings surfaced. All findings are nit or advisory-recommended severity. The three triage reservations are definitively addressed: (1) auth-surface -- auth-expert confirms the mixed-signal classification is correct: a single-URL 401-only host returns AUTH; mixed 401+404 returns MISSING; redirects and token audience are untouched thanks to allow_redirects=False. Addressed by: auth-expert confirms AUTH-vs-MISSING semantics are correct and redirect handling is unchanged. (2) security-surface -- supply-chain-security-expert confirms the slash-to-dash normalization matches GitLab's documented archive URL scheme and ref originates from local user config, not attacker input; no path-traversal or proxy-confusion vector. Addressed by: supply-chain-security-expert confirms ref is config-sourced and filename normalization cannot produce path-traversal. (3) multi-subsystem consistency -- python-architect confirms the ref-propagation priority chain is well-guarded and consistent across GitHub-family and GitLab paths. Addressed by: python-architect confirms consistent priority chain (version_spec > path_ref > source.ref > None) across both host families.

The doc-writer's recommended finding on CHANGELOG verbosity is the only non-nit item. It is valid editorial feedback but does not block merge -- CHANGELOG trimming is a style preference addressable in a squash-commit message or post-merge edit. The test-coverage-expert's recommended finding on missing integration-tier coverage is correctly tracked in #1918 and explicitly scoped out; the 9 passing unit tests provide adequate regression traps for the bounded changes.

No specialist disagreements exist. This is a clean, well-tested community contribution from a repeat enterprise contributor fixing real reliability bugs.

Aligned with: Portability by manifest: Restores correct --ref propagation so manifests referencing non-default branches resolve predictably across GitHub and GitLab hosts. Secure by default: allow_redirects=False prevents token leakage; AUTH-vs-MISSING reclassification improves diagnostic accuracy without weakening security posture. Multi-harness/multi-host: Fix ensures GitLab Artifactory proxy archives resolve correctly alongside GitHub-family paths.

Growth signal. Repeat community contributor from Check Point (chkp-roniz) -- 2 merged PRs fixing real GitLab enterprise pain. Consider a contributor spotlight in release notes and a 'Works with GitLab + Artifactory' callout in enterprise docs to signal active GitLab enterprise support.

Panel summary

Persona	B	R	N	Takeaway
Python Architect	0	0	2	Surgically correct fixes; auth-classification logic is sound, ref.replace('/','-') matches GitLab archive scheme, priority chain is well-guarded.
CLI Logging Expert	0	0	1	AUTH vs MISSING classification is semantically correct and actionable; one nit on RST double-backtick notation in error message constants.
DevX UX Expert	0	0	2	Solid UX fix restoring predictable --ref behavior; CHANGELOG is clear; AUTH vs MISSING reclassification improves recovery guidance.
Supply Chain Security Expert	0	0	2	No blocking security issues; filename normalization is scoped correctly; auth reclassification improves accuracy without weakening posture.
OSS Growth Hacker	0	0	1	Strong community contribution from a repeat GitLab enterprise contributor; 9 regression tests, CI green, clear CHANGELOG.
Auth Expert	0	0	1	_candidate_archive_status logic is sound; allow_redirects=False prevents token leakage; no auth regressions.
Doc Writer	0	1	2	Entries accurately describe regressions; primary concern is verbosity embedding implementation internals not needed in a user-facing CHANGELOG.
Test Coverage Expert	0	1	1	Both behavioral changes have unit-tier regression traps (9 pass); integration-tier gap for marketplace --ref propagation tracked in #1918.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 3 follow-ups

1. [Test Coverage Expert] Add integration-tier test for marketplace --ref propagation (tracked in test(marketplace): add integration-tier coverage for marketplace --ref end-to-end flow #1918) -- Marketplace install is a critical user-promise surface; unit mocks alone cannot catch HTTP-layer regressions in ref injection.
2. [Doc Writer] Trim CHANGELOG entries to user-observable impact; drop implementation internals -- APM CHANGELOG convention is one concise user-facing 'so what' per fix; current entries embed code mechanics.
3. [Python Architect] Extract _DEFAULT_REFS = frozenset({'main', 'HEAD'}) to eliminate dual-site drift risk (tracked in refactor(marketplace): extract ref-propagation helpers from resolve_marketplace_plugin #1917) -- Two-site duplication is a latent maintenance trap; a module-level constant makes the contract explicit.

Architecture

classDiagram
    direction LR
    class MarketplaceSource {
        <<ValueObject>>
        +name str
        +owner str
        +repo str
        +host str
        +ref str
    }
    class MarketplacePlugin {
        <<ValueObject>>
        +name str
        +source str | dict
    }
    class DependencyReference {
        <<ValueObject>>
        +reference str
        +virtual_path str
        +to_canonical() str
    }
    class resolve_marketplace_plugin {
        <<IOBoundary>>
        +resolve(name, mkt, version_spec) ResolvedPlugin
    }
    class build_artifactory_archive_url {
        <<Pure>>
        +build(host, prefix, owner, repo, ref, scheme) tuple
    }
    class _candidate_archive_status {
        <<IOBoundary>>
        +probe(host, prefix, owner, repo, ref) tuple
    }
    class _CandidateStatus {
        <<EnumLike>>
        +EXISTS str
        +MISSING str
        +AUTH str
        +INCONCLUSIVE str
    }
    resolve_marketplace_plugin ..> MarketplaceSource : reads ref
    resolve_marketplace_plugin ..> MarketplacePlugin : reads source
    resolve_marketplace_plugin ..> DependencyReference : builds
    _candidate_archive_status ..> _CandidateStatus : returns
    _candidate_archive_status ..> build_artifactory_archive_url : calls
    note for _candidate_archive_status "saw_auth AND NOT saw_non_auth_4xx => AUTH"
    note for resolve_marketplace_plugin "Priority: version_spec > path_ref > source.ref > None"
    class build_artifactory_archive_url:::touched
    class _candidate_archive_status:::touched
    class resolve_marketplace_plugin:::touched
    classDef touched fill:#fff3b0,stroke:#d47600

Loading

flowchart TD
    A["apm install plugin@marketplace"] --> B["resolve_marketplace_plugin()"]
    B --> C{"string source?"}
    C -->|Yes| D{"GitLab host?"}
    C -->|dict source| E["dict ref preserved"]
    D -->|Yes| F["dep_ref.ref = version_spec or path_ref or source.ref"]
    D -->|GitHub-family| G{"version_spec provided?"}
    G -->|Yes| H["canonical = base#version_spec"]
    G -->|No| I{"source.ref not in main/HEAD AND no # in canonical?"}
    I -->|Yes| J["canonical = canonical#source.ref"]
    I -->|No| K["canonical unchanged"]
    M["_candidate_archive_status()"] --> N["build_artifactory_archive_url()"]
    N --> O["HEAD probe each URL shape"]
    O --> P{"2xx?"}
    P -->|Yes| Q["return EXISTS"]
    P -->|No 401/403| R["saw_auth=True"]
    P -->|No other 4xx| S["saw_non_auth_4xx=True"]
    R --> T{"all URLs exhausted?"}
    S --> T
    T --> U{"saw_auth AND NOT saw_non_auth_4xx?"}
    U -->|Yes| V["return AUTH"]
    U -->|No| W["return MISSING"]

Loading

Recommendation

Zero blocking findings across 9 panelists. All three triage reservations confirmed resolved with evidence from auth-expert, supply-chain-security-expert, and python-architect. CI is green (13/13), 9 new regression tests pass, deferred work is tracked in #1917 and #1918. This is a clean, bounded community bug fix from a valued repeat contributor that restores correct behavior for GitLab enterprise users. Ship it.

---

Full per-persona findings

Python Architect

• [nit] Hardcoded default-ref guard could drift if more defaults are added at src/apm_cli/marketplace/resolver.py
  The set ('main', 'HEAD') appears in two places. If a third default ref (e.g. 'master') is ever added, both sites must be updated. A module-level constant _DEFAULT_REFS = frozenset({'main', 'HEAD'}) would make the contract explicit and searchable.
  Suggested: Extract _DEFAULT_REFS = frozenset({'main', 'HEAD'}) at module level and reference it in both guards.

• [nit] _CandidateStatus uses class attributes instead of enum at src/apm_cli/install/artifactory_resolver.py
  A stdlib enum.Enum would give typo-safety and IDE autocomplete. Not worth a refactor in this PR.
  Suggested: Future: migrate _CandidateStatus to enum.StrEnum for type safety.

CLI Logging Expert

• [nit] RST double-backtick notation in user-facing error messages renders as literal backticks in terminal at src/apm_cli/install/artifactory_resolver.py
  _ARTIFACTORY_BOUNDARY_UNRESOLVED and _ARTIFACTORY_BOUNDARY_AUTH use RST-style double-backtick quoting. In a terminal, the user sees literal backticks instead of clean quoting.
  Suggested: Replace double backticks with single backticks or bare quotes in error message constants.

DevX UX Expert

• [nit] CHANGELOG entry for the marketplace --ref fix includes implementation detail that users do not need at CHANGELOG.md
  Phrases like 'resolve_marketplace_plugin did not propagate source.ref' and 'Guards prevent double-injection' describe code mechanics, not user-observable behavior.
  Suggested: Trim to: 'apm install plugin@marketplace now correctly fetches from the marketplace registered --ref branch. Explicit per-plugin refs still override the marketplace-level ref.'

• [nit] Artifactory error message strings use RST double-backtick notation in a terminal context at src/apm_cli/install/artifactory_resolver.py
  Same issue as cli-logging-expert.
  Suggested: Use single backticks or plain quotes in CLI error messages.

Supply Chain Security Expert

• [nit] Pre-existing: ref not URL-encoded in Artifactory archive URL builder at src/apm_cli/utils/github_host.py
  build_artifactory_archive_url uses raw ref in URL path segments without urllib.parse.quote, unlike other URL builders in the same module. Pre-existing issue; not introduced by this PR. Low attack surface since ref originates from local user config.
  Suggested: Consider URL-encoding the ref in path segments in a follow-up PR for consistency with build_raw_content_url and build_ado_api_url.

• [nit] allow_redirects=False is good token-leak prevention at src/apm_cli/install/artifactory_resolver.py
  The HEAD probe correctly uses allow_redirects=False to prevent the Bearer token from leaking to redirect targets. Positive security pattern worth preserving.
  Suggested: No change needed -- noting as positive security practice.

OSS Growth Hacker

• [nit] Release note should call out GitLab enterprise unblock explicitly
  GitLab+Artifactory proxy users are a high-value enterprise cohort. The CHANGELOG entry is terse for this audience.
  Suggested: In release notes, surface GitLab enterprise proxy fix as a standalone bullet and shoutout Ron Izraeli (@chkp-roniz) as repeat community contributor.

Auth Expert

• [nit] Comment could clarify the mixed-signal rationale for future readers at src/apm_cli/install/artifactory_resolver.py:122
  The inline comment explains the 401+404 mixed case but does not cover the 404-first-then-401 ordering. An operator debugging a misconfigured proxy might wonder why they see MISSING instead of AUTH.
  Suggested: Expand comment: 'If any shape is definitively absent, the boundary is wrong regardless of auth errors on other shapes.'

Doc Writer

• [recommended] Both CHANGELOG entries are over-verbose with implementation internals at CHANGELOG.md
  Phrases like 'resolve_marketplace_plugin did not propagate source.ref', 'Guards prevent double-injection', 'ref injected into DependencyReference' are code mechanics. APM CHANGELOG convention is one concise user-facing 'so what' per fix.
  Suggested: Trim entry 1 to: 'apm install plugin@marketplace now respects the marketplace registered --ref branch. Previously, installs silently fell back to the repository default branch. Affects both GitHub-family and GitLab-hosted marketplaces. (Fix marketplace --ref not propagated to relative plugin sources (#1811) #1824)'. Trim entry 2 to: 'GitLab archive installs via PROXY_REGISTRY_ONLY=1 no longer return HTTP 404 when the branch name contains a slash (e.g. feat/my-feature). The proxy filename now uses a hyphen separator to match GitLab archive naming.'

• [nit] 'mirrors Fix marketplace --ref not propagated to relative plugin sources (#1811) #1824' is non-standard and ambiguous cross-reference at CHANGELOG.md
  Standard format uses '(#NNNN)' or 'fixes #NNNN'. 'mirrors' implies a duplicate rather than a related fix.
  Suggested: Replace with '(Fix marketplace --ref not propagated to relative plugin sources (#1811) #1824, by Ron Izraeli (@chkp-roniz))' or 'follow-on to Fix marketplace --ref not propagated to relative plugin sources (#1811) #1824 (by Ron Izraeli (@chkp-roniz))'.

• [nit] PROXY_REGISTRY_ONLY=1 cited in CHANGELOG without a docs pointer at CHANGELOG.md
  Referencing an undocumented env var in a CHANGELOG creates an implicit docs gap.
  Suggested: Add a brief inline gloss or link to the relevant reference page; if undocumented, file a follow-on issue.

Test Coverage Expert

• [recommended] Marketplace --ref propagation lacks integration-tier test (floor gap); tracked in test(marketplace): add integration-tier coverage for marketplace --ref end-to-end flow #1918 at src/apm_cli/marketplace/resolver.py
  The marketplace install pipeline is a critical user-promise surface. 7 unit tests mock the boundary but no integration-with-fixtures test invokes apm install end-to-end with a non-default --ref. Follow-up tracked in test(marketplace): add integration-tier coverage for marketplace --ref end-to-end flow #1918. Given thorough unit coverage, green CI, and tracked follow-up -- advisory not blocking.
  Proof (missing): tests/integration/marketplace/test_live_e2e.py -- proves: apm install plugin@marketplace fetches from registered --ref branch, not repo default [portability-by-manifest,devx]

• [nit] GitLab slash-ref filename fix has unit coverage only; integration probe would cement proxy scenario at src/apm_cli/utils/github_host.py
  Unit test verifies URL string construction but not the full HEAD-probe pipeline with a slash-ref. Low-risk since URL construction is pure string logic and unit assertion is strong.
  Proof (passed at unit): tests/unit/test_artifactory_support.py::test_gitlab_slash_ref_filename_normalised -- proves: GitLab archive filename replaces slashes with dashes for slash-containing branch refs [portability-by-manifest,devx]
  assert any(u.endswith('/-/archive/feat/my-slash-branch/repo-feat-my-slash-branch.zip') for u in urls)

Performance Expert -- inactive

Not active: no performance-relevant files touched; PR is a correctness bug fix with no hot-path changes, no perf claims, and no measurements.

_{This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.}