From 1ae5696c38bc373f5875cfd66eb625691125546b Mon Sep 17 00:00:00 2001 From: Sjors Robroek <51440732+danielmeppiel@users.noreply.github.com> Date: Sun, 12 Jul 2026 09:12:34 +0400 Subject: [PATCH 1/2] fix(deps): git-semver update idempotency in build_update_plan `apm update` reported a spurious UPDATE on every run for git-source semver dependencies already at their locked tag, and never converged: the lockfile was rewritten with the identical SHA each time (only timestamps changed). Root cause: the git-semver resolver rewrites `dep.reference` to the concrete tag and computes its SHA, but stashes the result in `ctx.git_semver_resolutions` (and the download result) without attaching it back to `dep.resolved_reference`. At plan time `resolved_reference` is None, so `build_update_plan` compares the locked commit against `None` and classifies every git-semver dep as `update`. Fix mirrors the registry idempotency fix in #1908, applied to the git-source path: when a dep has no freshly-resolved commit, the locked entry pins an immutable tag (`resolved_tag` set), and the ref is unchanged, borrow the locked commit so the comparison sees `unchanged`. Scoped to tags -- a branch tip can advance under a stable ref name, so branch deps (no `resolved_tag`) still surface real updates. Adds two regression tests: cached git-semver tag stays `unchanged`; branch-tip advance still shows `update`. --- CHANGELOG.md | 11 +++++ src/apm_cli/install/plan.py | 18 ++++++++ tests/unit/install/test_plan.py | 73 +++++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2530ba47..539782783 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- `apm update` no longer reports a spurious update on every run for git-source + semver dependencies already at their locked tag. The git-semver resolver + rewrites the dep reference to the concrete tag but does not attach the + resolved SHA to `resolved_reference`, so the update plan compared the locked + commit against `None` and never converged. `build_update_plan` now borrows the + locked commit for cached, immutable-tag deps whose ref is unchanged, mirroring + the registry fix in #1908 (branch deps are unaffected -- their tips can still + advance under a stable ref name). (closes microsoft/apm#2163) + ## [0.25.0] - 2026-07-12 ### Added diff --git a/src/apm_cli/install/plan.py b/src/apm_cli/install/plan.py index e02a9880f..a7f71e5e9 100644 --- a/src/apm_cli/install/plan.py +++ b/src/apm_cli/install/plan.py @@ -220,6 +220,24 @@ def build_update_plan( ): new_ref = old.resolved_ref + # Cached git-semver dep already at its locked tag: the resolver rewrote + # ``dep.reference`` to the concrete tag but stashed the resolved SHA in + # ``ctx.git_semver_resolutions`` without attaching it to + # ``dep.resolved_reference``, so ``new_commit`` is None here. Borrow the + # locked commit when the ref is unchanged AND the locked entry pins an + # immutable tag (``resolved_tag`` set) -- otherwise every ``apm update`` + # would compare the locked SHA against None and emit a spurious UPDATE + # that never converges (git-source parity with the registry fix in + # #1908). Scoped to tags: a branch tip can advance under a stable ref + # name, so branch deps (no ``resolved_tag``) must still surface updates. + if ( + new_commit is None + and old is not None + and getattr(old, "resolved_tag", None) + and new_ref == old.resolved_ref + ): + new_commit = old.resolved_commit + if old is None: plan_entries.append( PlanEntry( diff --git a/tests/unit/install/test_plan.py b/tests/unit/install/test_plan.py index e6fd6fc09..5a9058271 100644 --- a/tests/unit/install/test_plan.py +++ b/tests/unit/install/test_plan.py @@ -235,6 +235,79 @@ def test_source_transition_to_registry_is_not_masked_as_unchanged(self): assert entry.new_resolved_ref != "v1.0.0" assert entry.action == "update" + def test_git_semver_tag_dep_unchanged_when_cached_and_ref_matches(self): + """A cached git-semver dep already at its locked tag must stay 'unchanged'. + + Regression (git-source parity with #1908's registry fix): on ``apm update`` + the git-semver resolver rewrites ``dep.reference`` to the concrete tag and + computes its SHA, but that SHA is stashed in ``ctx.git_semver_resolutions`` + and never attached back to ``dep.resolved_reference``. So at plan time the + dep carries ``reference='eli5--v2.0.0'`` with ``resolved_reference=None``. + Without the locked-commit fallback, ``build_update_plan`` compares the + locked SHA against ``None`` and emits a spurious UPDATE on every run -- the + lockfile then rewrites the identical SHA, so ``apm update`` never converges. + + The locked entry carries a concrete ``resolved_tag`` (immutable), so the + matching-ref case is safe to treat as unchanged. + """ + lock = _new_lockfile() + lock.add_dependency( + LockedDependency( + repo_url="srobroek/agentic-packages", + resolved_ref="eli5--v2.0.0", + resolved_commit="9" * 40, + depth=1, + is_virtual=True, + virtual_path="packages/eli5", + constraint=">=2.0.0 <3.0.0", + resolved_tag="eli5--v2.0.0", + ) + ) + # Cached git-semver dep: ref rewritten to the concrete tag by the resolver, + # but resolved_reference never populated (SHA not plumbed to the plan). + dep = DependencyReference( + repo_url="srobroek/agentic-packages", + reference="eli5--v2.0.0", + is_virtual=True, + virtual_path="packages/eli5", + ) + assert getattr(dep, "resolved_reference", None) is None + + plan = build_update_plan(lock, [dep]) + + assert plan.has_changes is False + entry = plan.entries[0] + assert entry.action == "unchanged" + assert entry.new_resolved_ref == "eli5--v2.0.0" + # The locked commit is borrowed so the display shows a real SHA, not '-'. + assert entry.new_resolved_commit == "9" * 40 + + def test_git_branch_dep_tip_advance_still_shows_update(self): + """A branch dep whose tip advanced must NOT be masked as unchanged. + + Guards the locked-commit fallback: it applies only to immutable tag refs + (locked entry carries ``resolved_tag``). A branch dep has no ``resolved_tag`` + and its tip can move under a stable ref name, so a freshly-resolved commit + that differs from the lockfile must still surface as an update. + """ + lock = _new_lockfile() + lock.add_dependency( + LockedDependency( + repo_url="https://github.com/o/r", + resolved_ref="main", + resolved_commit="a" * 40, + depth=1, + ) + ) + # Branch dep, freshly resolved to a new tip SHA (no resolved_tag on lock). + deps = [_resolved_dep("https://github.com/o/r", "main", "b" * 40)] + + plan = build_update_plan(lock, deps) + + assert plan.has_changes is True + assert plan.entries[0].action == "update" + assert plan.entries[0].new_resolved_commit == "b" * 40 + # ----------------------------------------------------------------------------- # render_plan_text From 6d7d0eb142d753bc4e81aa75f7cd91e0f73d0dda Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Sun, 12 Jul 2026 16:58:07 +0200 Subject: [PATCH 2/2] docs: clarify git-semver idempotency fix Focus the release note on user-visible convergence and describe the locked SHA as the integrity anchor rather than treating git tags as immutable. Addresses panel follow-ups from OSS growth, docs, and supply-chain review. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 11 +++-------- src/apm_cli/install/plan.py | 16 ++++++---------- tests/unit/install/test_plan.py | 13 +++++++------ 3 files changed, 16 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 539782783..f494a61c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,14 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- `apm update` no longer reports a spurious update on every run for git-source - semver dependencies already at their locked tag. The git-semver resolver - rewrites the dep reference to the concrete tag but does not attach the - resolved SHA to `resolved_reference`, so the update plan compared the locked - commit against `None` and never converged. `build_update_plan` now borrows the - locked commit for cached, immutable-tag deps whose ref is unchanged, mirroring - the registry fix in #1908 (branch deps are unaffected -- their tips can still - advance under a stable ref name). (closes microsoft/apm#2163) +- `apm update` now converges for git-source semver dependencies already at + their locked tag instead of reporting a spurious update on every run. Branch + dependencies remain unaffected. (by @srobroek, #2165) ## [0.25.0] - 2026-07-12 diff --git a/src/apm_cli/install/plan.py b/src/apm_cli/install/plan.py index a7f71e5e9..1d3a8fe5d 100644 --- a/src/apm_cli/install/plan.py +++ b/src/apm_cli/install/plan.py @@ -220,16 +220,12 @@ def build_update_plan( ): new_ref = old.resolved_ref - # Cached git-semver dep already at its locked tag: the resolver rewrote - # ``dep.reference`` to the concrete tag but stashed the resolved SHA in - # ``ctx.git_semver_resolutions`` without attaching it to - # ``dep.resolved_reference``, so ``new_commit`` is None here. Borrow the - # locked commit when the ref is unchanged AND the locked entry pins an - # immutable tag (``resolved_tag`` set) -- otherwise every ``apm update`` - # would compare the locked SHA against None and emit a spurious UPDATE - # that never converges (git-source parity with the registry fix in - # #1908). Scoped to tags: a branch tip can advance under a stable ref - # name, so branch deps (no ``resolved_tag``) must still surface updates. + # Cached git-semver dep at its locked tag: the resolver rewrote + # ``dep.reference`` but did not attach the resolved SHA to + # ``dep.resolved_reference``, so ``new_commit`` is None. If the ref still + # matches a tag-pinned lock entry, borrow its locked SHA, which remains + # the integrity anchor. ``resolved_tag`` distinguishes tag resolutions + # from movable branches, whose freshly resolved SHAs must remain visible. if ( new_commit is None and old is not None diff --git a/tests/unit/install/test_plan.py b/tests/unit/install/test_plan.py index 5a9058271..0ed2efa6d 100644 --- a/tests/unit/install/test_plan.py +++ b/tests/unit/install/test_plan.py @@ -247,8 +247,8 @@ def test_git_semver_tag_dep_unchanged_when_cached_and_ref_matches(self): locked SHA against ``None`` and emits a spurious UPDATE on every run -- the lockfile then rewrites the identical SHA, so ``apm update`` never converges. - The locked entry carries a concrete ``resolved_tag`` (immutable), so the - matching-ref case is safe to treat as unchanged. + The locked entry carries the concrete ``resolved_tag`` produced by semver + resolution, so the matching-ref case is safe to treat as unchanged. """ lock = _new_lockfile() lock.add_dependency( @@ -285,10 +285,11 @@ def test_git_semver_tag_dep_unchanged_when_cached_and_ref_matches(self): def test_git_branch_dep_tip_advance_still_shows_update(self): """A branch dep whose tip advanced must NOT be masked as unchanged. - Guards the locked-commit fallback: it applies only to immutable tag refs - (locked entry carries ``resolved_tag``). A branch dep has no ``resolved_tag`` - and its tip can move under a stable ref name, so a freshly-resolved commit - that differs from the lockfile must still surface as an update. + Guards the locked-commit fallback: it applies only when no fresh SHA exists + and the locked entry carries ``resolved_tag``. A branch dep has no + ``resolved_tag`` and its tip can move under a stable ref name, so a freshly + resolved commit that differs from the lockfile must still surface as an + update. """ lock = _new_lockfile() lock.add_dependency(