Skip to content

fix: preserve DependencyReference through download pipeline (#382)#383

Merged
danielmeppiel merged 3 commits intomicrosoft:mainfrom
sergio-sisternes-epam:fix/382-generic-host-subdirectory-packages
Mar 20, 2026
Merged

fix: preserve DependencyReference through download pipeline (#382)#383
danielmeppiel merged 3 commits intomicrosoft:mainfrom
sergio-sisternes-epam:fix/382-generic-host-subdirectory-packages

Conversation

@sergio-sisternes-epam
Copy link
Collaborator

Summary

Fixes #382 — Subdirectory packages fail on non-GitHub/ADO hosts (generic git hosts like GitLab, Gitea, Bitbucket Server).

Root Cause

The download pipeline converted DependencyReference to a string via str() and re-parsed it in download_package(). The str()parse() round-trip lost the repo_url / virtual_path boundary on generic hosts, where _detect_virtual_package() uses min_base_segments = len(path_segments). This caused APM to clone an invalid URL with the subdirectory baked into the repo path:

# Broken: tries to clone nonexistent repo
git clone -- https://git.example.com/org/repo/dist/subdir

# Expected: clone base repo, extract subdir
git clone -- https://git.example.com/org/repo

Reproduced end-to-end against a self-hosted GitLab instance.

Fix

Pass the structured DependencyReference object through the pipeline instead of flattening to string (Option A from the issue):

  • download_package() and resolve_git_reference() now accept Union[str, DependencyReference], skipping the parse when already structured
  • build_download_ref() returns a DependencyReference (via dataclasses.replace()) instead of a flat string
  • All call sites in install.py and _utils.py updated to pass the object directly

Changes

File Change
src/apm_cli/deps/github_downloader.py download_package() and resolve_git_reference() accept Union[str, DependencyReference]
src/apm_cli/drift.py build_download_ref() returns DependencyReference via dataclasses.replace()
src/apm_cli/commands/install.py Transitive dep callback passes object directly; cache-check uses object
src/apm_cli/commands/deps/_utils.py Removed str() wrapping in update commands
tests/test_apm_package_models.py Added regression tests for generic host round-trip
tests/unit/test_install_update.py Updated build_download_ref tests to assert on object properties

Testing

  • All 2583 existing tests pass
  • New TestGenericHostSubdirectoryRoundTrip class with parametrized tests covering GitLab, Gitea, Bitbucket, and the reporter's exact scenario
  • Verified fix preserves virtual_path for the reporter's exact case (git.example.com/ai/grandpa-s-skills with dist/brain-council)

…t#382)

Subdirectory packages on non-GitHub/ADO hosts (e.g. GitLab, Gitea) failed
because the download pipeline converted DependencyReference to a string
via str() and re-parsed it in download_package(). The str() → parse()
round-trip lost the repo_url / virtual_path boundary on generic hosts
where _detect_virtual_package() uses a dynamic min_base_segments heuristic.

Fix: pass the structured DependencyReference object through the pipeline
instead of flattening to string. Specifically:

- download_package() and resolve_git_reference() now accept
  Union[str, DependencyReference], skipping the parse when already
  structured
- build_download_ref() returns a DependencyReference (via
  dataclasses.replace) instead of a flat string
- All call sites in install.py and _utils.py updated to pass the
  object directly

Closes microsoft#382

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

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes installs of subdirectory (virtual) packages on non-GitHub/ADO git hosts by preserving the structured DependencyReference object through the download pipeline, avoiding a lossy str()parse() round-trip that could collapse repo_url and virtual_path.

Changes:

  • build_download_ref() now returns a DependencyReference (with reference overridden via dataclasses.replace() when using a locked SHA).
  • GitHubPackageDownloader.download_package() and .resolve_git_reference() now accept Union[str, DependencyReference] and skip parsing when already structured.
  • Call sites updated to pass DependencyReference directly; tests updated/added for the new object-based behavior.

Reviewed changes

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

Show a summary per file
File Description
src/apm_cli/deps/github_downloader.py Accepts structured refs in download_package/resolve_git_reference to avoid re-parsing and preserve virtual_path.
src/apm_cli/drift.py Changes build_download_ref() to return DependencyReference instead of a flattened string.
src/apm_cli/commands/install.py Updates transitive download callback + cache pre-resolution to pass DependencyReference objects.
src/apm_cli/commands/deps/_utils.py Updates deps update commands to stop wrapping deps with str() before download.
tests/test_apm_package_models.py Adds regression tests related to generic-host subdirectory refs.
tests/unit/test_install_update.py Updates existing tests to assert on DependencyReference fields rather than string formatting.

- Normalize original_ref to string in resolve_git_reference() to avoid
  leaking DependencyReference objects into ResolvedReference.original_ref
- Replace shallow test with one that verifies DependencyReference.parse()
  is NOT called when passing a structured object to download_package()
- Update test_resolve_git_reference_branch to match normalized original_ref

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel danielmeppiel merged commit 379b9d3 into microsoft:main Mar 20, 2026
7 checks passed
@sergio-sisternes-epam sergio-sisternes-epam deleted the fix/382-generic-host-subdirectory-packages branch March 20, 2026 14:38
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] Subdirectory packages fail on non-GitHub/ADO hosts (generic git hosts)

3 participants