Skip to content

fix: respect update_refs in download_callback and skip logic (#548)#550

Merged
danielmeppiel merged 4 commits into
microsoft:mainfrom
sergio-sisternes-epam:fix/update-refs-already-resolved
Apr 2, 2026
Merged

fix: respect update_refs in download_callback and skip logic (#548)#550
danielmeppiel merged 4 commits into
microsoft:mainfrom
sergio-sisternes-epam:fix/update-refs-already-resolved

Conversation

@sergio-sisternes-epam
Copy link
Copy Markdown
Collaborator

Summary

Fixes #548 -- apm deps update now correctly re-resolves transitive
dependencies instead of silently reusing stale locked SHAs from the lockfile.

Problem

When running apm deps update, transitive dependencies could be skipped
in two ways:

  1. download_callback() (BFS resolution) pinned transitive deps to
    the old locked SHA from apm.lock.yaml, ignoring update_refs=True.
  2. Sequential loop let already_resolved (set by the callback)
    unconditionally bypass SHA comparison, so even when the remote had a
    newer commit the download was skipped.

The net effect: apm deps update would report "All packages already at
latest refs" while transitive dependencies remained at stale commits.

Fix

Two surgical changes in install.py (+3/-3 lines):

  • Line 1268: if locked_ref: -> if locked_ref and not update_refs:
    -- callback uses the manifest ref (latest) during --update instead of
    the locked SHA.
  • Line 1920: already_resolved -> (already_resolved and not update_refs)
    -- forces SHA comparison in the sequential loop during --update.

Normal install behavior (update_refs=False) is completely unchanged.

Testing

  • 27 unit tests covering both changed code paths with an exhaustive
    truth table (test_install_update_refs.py)
  • Full unit suite: 3,545 tests passing
  • End-to-end validation: installed a package with 2 transitive deps,
    pushed a new commit upstream, confirmed apm deps update re-downloaded
    all 3 packages at the new SHA

Documentation

  • dependencies.md: clarified that --update re-resolves transitive deps
  • cli-commands.md: specified "direct and transitive" in deps update
    description
  • CHANGELOG.md: entry under [Unreleased] > Fixed

Changed files

File Change
src/apm_cli/commands/install.py Gate locked_ref and already_resolved with not update_refs
tests/unit/test_install_update_refs.py 27 tests for skip_download and locked-ref logic
docs/src/content/docs/guides/dependencies.md Clarify transitive re-resolution
docs/src/content/docs/reference/cli-commands.md Add "direct and transitive"
CHANGELOG.md Unreleased entry

Sergio Sisternes and others added 2 commits April 2, 2026 18:25
…ft#548)

When running `apm deps update`, the BFS download_callback was using
locked SHAs from the lockfile for transitive dependencies instead of
resolving the latest ref. Additionally, packages marked as
already_resolved in the callback unconditionally skipped SHA comparison
in the sequential loop.

Two changes:
- download_callback: gate locked_ref usage with `not update_refs`
- sequential loop: gate already_resolved skip with `not update_refs`

Normal install behavior (update_refs=False) is unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rosoft#548)

- dependencies.md: clarify --update re-resolves transitive deps
- cli-commands.md: specify 'direct and transitive' in deps update description
- test_install_update_refs.py: expand from 17 to 27 tests with exhaustive
  truth table and improved docstrings (architect review)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 2, 2026 17:25
Copy link
Copy Markdown
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

Fixes apm deps update behavior so --update (update_refs=True) does not reuse stale locked SHAs for transitive dependencies during resolution/download, aligning update behavior with expected “re-resolve latest refs” semantics.

Changes:

  • Gate download_callback()’s lockfile SHA override behind not update_refs so updates use the manifest ref instead of the locked SHA.
  • Gate sequential-loop already_resolved skip logic behind not update_refs so --update forces SHA comparison instead of blindly skipping.
  • Add unit tests and update docs/changelog to describe “direct and transitive” update behavior.

Reviewed changes

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

Show a summary per file
File Description
src/apm_cli/commands/install.py Adjusts locked-ref selection and skip logic to respect update_refs=True.
tests/unit/test_install_update_refs.py Adds tests for the two updated boolean conditions.
docs/src/content/docs/guides/dependencies.md Clarifies that --update re-resolves transitive dependencies (refs) as well.
docs/src/content/docs/reference/cli-commands.md Updates deps update description to explicitly include transitive deps.
CHANGELOG.md Adds an [Unreleased] > Fixed entry for the bugfix.

Comment thread src/apm_cli/commands/install.py
Comment thread src/apm_cli/commands/install.py Outdated
Comment thread tests/unit/test_install_update_refs.py
Comment thread tests/unit/test_install_update_refs.py Outdated
- Break skip_download expression across multiple lines (install.py)
- Wrap parametrize argument tuple across lines (test file)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@danielmeppiel danielmeppiel merged commit 86762ca into microsoft:main Apr 2, 2026
7 checks passed
@sergio-sisternes-epam sergio-sisternes-epam deleted the fix/update-refs-already-resolved branch April 3, 2026 18:16
sergio-sisternes-epam added a commit that referenced this pull request May 19, 2026
…550)

* fix: respect update_refs in download_callback and skip logic (#548)

When running `apm deps update`, the BFS download_callback was using
locked SHAs from the lockfile for transitive dependencies instead of
resolving the latest ref. Additionally, packages marked as
already_resolved in the callback unconditionally skipped SHA comparison
in the sequential loop.

Two changes:
- download_callback: gate locked_ref usage with `not update_refs`
- sequential loop: gate already_resolved skip with `not update_refs`

Normal install behavior (update_refs=False) is unchanged.

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

* docs: clarify transitive dep re-resolution; expand test coverage (#548)

- dependencies.md: clarify --update re-resolves transitive deps
- cli-commands.md: specify 'direct and transitive' in deps update description
- test_install_update_refs.py: expand from 17 to 27 tests with exhaustive
  truth table and improved docstrings (architect review)

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

* style: address Copilot review formatting feedback

- Break skip_download expression across multiple lines (install.py)
- Wrap parametrize argument tuple across lines (test file)

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

---------

Co-authored-by: Sergio Sisternes <sergio.sisternes@epam.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Daniel Meppiel <51440732+danielmeppiel@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 deps update skips transitive deps due to download_callback using locked SHA

3 participants