Skip to content

Fix marketplace install ignoring GitLab host for url-type sources (#1848)#1853

Merged
danielmeppiel merged 3 commits into
mainfrom
sergio-sisternes-epam-fix-marketplace-install-gitlab-host-pres
Jun 20, 2026
Merged

Fix marketplace install ignoring GitLab host for url-type sources (#1848)#1853
danielmeppiel merged 3 commits into
mainfrom
sergio-sisternes-epam-fix-marketplace-install-gitlab-host-pres

Conversation

@sergio-sisternes-epam

Copy link
Copy Markdown
Collaborator

Description

When installing a package via apm install pkg@marketplace where the marketplace is on GitLab, the CLI silently discards the GitLab host from the resolved URL and defaults to github.com for auth resolution. The marketplace cache correctly stores the full GitLab URL, but the install path loses the host during the resolution step.

The root cause is in resolve_marketplace_plugin(): for plugins with a url-type source on non-GitHub-family hosts, _resolve_url_source() strips the host from the canonical, and no downstream block rebuilds a structured DependencyReference to restore it. The existing host-backfill only covers GitHub-family enterprise hosts (.ghe.com).

This PR adds a new block that builds a structured DependencyReference directly from the plugin's URL when the marketplace is on a non-GitHub-family host (GitLab, generic git) and the plugin source type is "url". This preserves the host through to auth resolution.

Fixes: #1848

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Maintenance / refactor

Testing

  • Tested locally
  • All existing tests pass
  • Added tests for new functionality (if applicable)

Three new test cases added:

  • test_url_source_on_gitlab_preserves_host -- verifies dep_ref is built with gitlab.com host
  • test_url_source_on_gitlab_with_version_spec_override -- verifies version_spec overrides source ref
  • test_url_source_on_github_no_dep_ref -- confirms github.com marketplace behaviour unchanged

Spec conformance (OpenAPM v0.1)

  • N/A -- this PR does not change OpenAPM-observable behaviour.

)

When a marketplace plugin uses a url-type source on a non-GitHub-family
host (GitLab, generic git), the resolver now builds a structured
DependencyReference from the plugin URL. This preserves the host through
to auth resolution instead of silently falling back to github.com.

The fix mirrors the existing _gitlab_in_marketplace_dependency_reference
pattern but applies to url-type sources where the plugin's own URL
already carries the full clone target.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 19, 2026 17:59
Comment thread tests/unit/marketplace/test_marketplace_resolver.py Fixed
@sergio-sisternes-epam sergio-sisternes-epam added the panel-review Trigger the apm-review-panel gh-aw workflow label Jun 19, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes marketplace installs where a GitLab-hosted marketplace contains url-type plugin sources: the resolved canonical string (and auth routing) should preserve the non-GitHub host instead of silently defaulting to github.com.

Changes:

  • Builds a structured DependencyReference from a plugin’s url source for non-GitHub-family marketplace hosts (e.g., GitLab/generic) to preserve host through auth resolution.
  • Adds unit tests covering GitLab host preservation, version_spec override behavior, and ensuring GitHub marketplace behavior is unchanged.

Reviewed changes

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

File Description
src/apm_cli/marketplace/resolver.py Adds a new resolution branch to construct a dep ref from url-type plugin sources on non-GitHub marketplace kinds to preserve host in canonical/auth routing.
tests/unit/marketplace/test_marketplace_resolver.py Adds regression tests for GitLab-hosted marketplaces with url sources and a guard test for GitHub marketplaces.

Comment on lines +848 to +856
_ref = plugin.source.get("ref", "")
_effective_ref = version_spec or (
_ref.strip() if isinstance(_ref, str) and _ref.strip() else ""
)
_entry: dict = {"git": _url}
if _effective_ref:
_entry["ref"] = _effective_ref
dep_ref = DependencyReference.parse_from_dict(_entry)
canonical = dep_ref.to_canonical()
Comment thread tests/unit/marketplace/test_marketplace_resolver.py Outdated
Sergio Sisternes and others added 2 commits June 20, 2026 12:42
Use exact dependency fields for the GitLab URL-source regression test so the host preservation guard is precise and CodeQL-safe. Also removes the issue-number suffix from the new debug log message per the panel UX fold.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add the required Unreleased changelog entry for the marketplace URL host preservation fix, crediting the community author and linked issue.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel

Copy link
Copy Markdown
Collaborator

APM Review Panel: ship_with_followups

This PR now preserves GitLab marketplace URL-source hosts through install/auth routing, with exact regression coverage and a changelog entry.

cc @sergio-sisternes-epam -- a fresh advisory pass is ready for your review.

The panel converged on the core fix: the resolver now carries the non-GitHub host in a structured DependencyReference, and the added regression tests prove the host, repo path, ref, and canonical value exactly. Follow-up folds tightened the URL assertions to avoid substring-style URL checks, removed tracker-only wording from the debug log, and added the required Unreleased changelog entry. Remaining suggestions are broader resolver/auth ergonomics, so they are deferred rather than expanding this bug fix.

Aligned with: multi-harness/multi-host support: GitLab marketplace installs no longer re-route to github.com; pragmatic as npm: the fix stays local to marketplace resolution with targeted tests.

Panel summary

Persona B R N Takeaway
Python Architect 0 0 1 Core fix is localized; broader resolver extraction is optional follow-up.
CLI Logging Expert 0 0 0 No user-facing CLI output changed; debug wording was folded.
DevX UX Expert 0 0 0 Install behavior now matches the user's marketplace host expectation.
Supply Chain Security Expert 0 0 1 Token routing is scoped to the resolved host; broader auth diagnostics can follow later.
OSS Growth Hacker 0 0 0 Community GitLab fix is now represented in the changelog.
Auth Expert 0 0 1 Host preservation reaches AuthResolver; self-hosted auth refinements are separate scope.
Test Coverage Expert 0 0 0 Regression tests now assert exact host/repo/ref/canonical values.

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

Recommendation

Ready for maintainer review. CI is green on the latest head, the in-scope panel items were folded, and the remaining notes cross into broader resolver/auth refactoring rather than the #1848 host-preservation bug.


Folded in this run

  • (panel) Tightened the GitLab URL-source regression test to exact DependencyReference and canonical assertions -- resolved in 060c704e1122a1f2b16f6ab5409f5d18d5e2e120.
  • (panel) Removed the issue-number suffix from the new debug log wording -- resolved in 060c704e1122a1f2b16f6ab5409f5d18d5e2e120.
  • (panel) Added the required Unreleased changelog entry crediting @sergio-sisternes-epam -- resolved in 2432b4d568b83be452647edcb48dbc52407b8002.

Deferred (out-of-scope follow-ups)

Regression-trap evidence (mutation-break gate)

  • tests/unit/marketplace/test_marketplace_resolver.py::TestResolveMarketplacePluginGitLabMonorepo::test_url_source_on_gitlab_preserves_host -- deleted dep_ref is None and _source_needs_explicit_git_path(source) guard; test FAILED as expected; guard restored.

Lint contract

uv run --extra dev ruff check src/ tests/ and
uv run --extra dev ruff format --check src/ tests/ both silent.
Additional auth-surface guard: bash scripts/lint-auth-signals.sh passed. Duplication guard: pylint R0801 passed.

CI

gh pr checks 1853 --repo microsoft/apm --watch observed all checks passing on head 2432b4d568b83be452647edcb48dbc52407b8002 (after 0 CI fix iteration(s)). Green run set includes Lint, Build & Test shards, Coverage Combine, CodeQL, APM Self-Check, NOTICE Drift Check, Spec conformance gate, PR Binary Smoke, and license/cla.

Mergeability status

Captured from gh pr view 1853 --json mergeable,mergeStateStatus,statusCheckRollup immediately after the last push of this run.

PR head SHA CEO stance iters folds defers Copilot rounds CI mergeable mergeStateStatus notes
#1853 2432b4d ship_with_followups 1 3 2 2 green MERGEABLE BLOCKED pending required review

Convergence

1 outer iteration(s); 2 Copilot round(s). Final panel stance: ship_with_followups.

Ready for maintainer review.

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

@danielmeppiel danielmeppiel removed the panel-review Trigger the apm-review-panel gh-aw workflow label Jun 20, 2026
@danielmeppiel danielmeppiel merged commit f94ff92 into main Jun 20, 2026
14 checks passed
@danielmeppiel danielmeppiel deleted the sergio-sisternes-epam-fix-marketplace-install-gitlab-host-pres branch June 20, 2026 17:41
danielmeppiel added a commit that referenced this pull request Jun 20, 2026
…#1871)

Add the user-facing PRs merged after the v0.21.0 tag that were missing
from the changelog (#1830 Added; #1854, #1856 Fixed), add the external-
contributor credit on #1855, and move #1853 out of the released [0.21.0]
section into [Unreleased] since it merged after the v0.21.0 tag. No version
bump -- everything stays under [Unreleased].

Co-authored-by: danielmeppiel <danielmeppiel@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] apm install <pkg>@<marketplace> ignores GitLab host from marketplace cache, falls back to github.com

4 participants