fix(deps): git-semver update idempotency in build_update_plan

[srobroek]

Summary

apm update (and apm update -g) reports a spurious UPDATE on every run for git-source semver dependencies that are already at their locked tag, and never converges. Re-running produces the same "N updated" line indefinitely; the lockfile is rewritten with the identical commit SHA each time (only generated_at / resolved_at timestamps change), and no deployed files change on disk.

This is the git-source counterpart of the registry idempotency bug fixed for registry deps in #1908 — that fix did not cover git-semver deps.

Root cause

On apm update, the git-semver resolver (_maybe_resolve_git_semver in install/phases/resolve.py) rewrites dep.reference to the concrete tag and computes its SHA (GitSemverResolution.resolved_sha). But the SHA is stashed in ctx.git_semver_resolutions and the download result — it is never attached back to dep.resolved_reference.

build_update_plan (called from install/pipeline.py right after resolve, before download) reads dep.resolved_reference, which is None at that point. So _extract_new_ref_and_commit returns the correct concrete tag for the ref but None for the commit. The classifier then compares:

old_commit (real locked SHA)  vs  new_commit (None)   ->  UPDATE

even though old_ref == new_ref (same concrete tag). Verified by instrumenting build_update_plan during a live update:

PLAN-TIME DEP STATE
  key=srobroek/agentic-packages/packages/eli5 reference='eli5--v2.0.0' resolved_reference=None
PLAN ENTRIES
  update: ...eli5 old_commit='69148d93...' new_commit=None old_ref='eli5--v2.0.0' new_ref='eli5--v2.0.0'

Fix

Mirror #1908's registry approach on the git-source path. In build_update_plan, when a dep has no freshly-resolved commit, the locked entry pins an immutable tag (resolved_tag is set), and the ref is unchanged, borrow the locked commit so the comparison correctly yields unchanged.

Scoped deliberately to tags: a branch tip can advance under a stable ref name, so branch deps (no resolved_tag) must still surface real updates. The existing registry fallback immediately above is left untouched.

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

Tests

• test_git_semver_tag_dep_unchanged_when_cached_and_ref_matches — the reproduction; fails before the fix, passes after.
• test_git_branch_dep_tip_advance_still_shows_update — guards that a real branch-tip advance is not masked.
• Full tests/unit/install/ and tests/unit/deps/ suites green post-rebase onto current main.

End-to-end verification

Against a real git-semver dep (srobroek/agentic-packages/packages/eli5#>=2.0.0 <3.0.0), built from this branch:

• Before: every apm update → Updated N APM dependencies (lockfile diff = timestamps only).
• After: first update annotates the lockfile once; run 2 and run 3 → All dependencies already at their latest matching refs. — converged.

Closes #2163

[Copilot]

Pull request overview

Fixes apm update idempotency for git-source semver dependencies that are already locked to an immutable tag, preventing repeated “update” results when nothing has changed.

Changes:

• Adjusts build_update_plan to treat cached git-semver tag deps as unchanged by borrowing the locked commit when the resolved ref is unchanged and the lockfile pins a resolved_tag.
• Adds regression tests covering (1) the cached git-semver tag unchanged case and (2) ensuring branch-tip advances still report updates.
• Adds an Unreleased changelog entry describing the fix.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/apm_cli/install/plan.py	Adds a locked-commit fallback for cached git-semver tag deps to prevent spurious updates.
tests/unit/install/test_plan.py	Adds unit tests to reproduce the bug and guard against masking branch updates.
CHANGELOG.md	Documents the fix under Unreleased.

[danielmeppiel]

APM Review Panel: ship_with_followups

Fixes non-converging apm update for git-source semver deps pinned at their locked tag, restoring idempotent update behavior.

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

The panel converges cleanly on ship. Six of eight active panelists returned valid findings or explicit no-op signals. CLI-logging, DevX-UX, performance, and auth (inactive) all confirm zero surface-area impact -- this is a pure data-path correctness fix inside build_update_plan that eliminates a spurious UPDATE report for git-semver deps already at their locked commit. Supply-chain-security confirms the trust model is sound: the locked SHA, not the tag name, is the integrity anchor. The oss-growth-hacker and doc-writer both flag the same issue -- the CHANGELOG entry leaks internal Python symbols and an unexplained cross-reference into user-facing release notes. That is a valid readability concern and the single actionable follow-up.

Two panelists -- python-architect and test-coverage-expert -- returned schema-invalid responses after retries and are recorded as schema failures. Their findings, if any, are unvalidated and carry zero weight in this synthesis. The test-coverage-expert reportedly raised an integration-tier convergence concern, but because the return never passed schema validation, it cannot be treated as a validated finding. The maintainer may choose to investigate that concern independently, but this panel does not elevate it. The two included unit tests exercise the guarded path directly; absent a validated coverage finding, the test surface is adequate for a bug fix of this scope.

Strategically this is a minor but trust-critical fix. Idempotent update behavior is table-stakes for any package manager; users who see phantom updates on every run lose confidence in the resolver. Shipping this promptly with a tightened CHANGELOG entry reinforces APM's reliability story.

Dissent. No dissent between validated panelists. The oss-growth-hacker and doc-writer independently flagged the same CHANGELOG clarity issue, reinforcing each other. The two schema-failure panelists produced no validated findings to weigh.

Aligned with: Pragmatic as npm: Update idempotency is baseline PM hygiene; this fix closes a gap that would erode user trust in apm update. Secure by default: Locked SHA remains the trust anchor for cached deps; no tag-deref bypass introduced.

Growth signal. The CHANGELOG entry matters for upgrade-and-share conversion. A compact symptom/outcome statement -- "apm update now converges for git-source semver deps already at their locked tag" -- is the kind of one-liner that lands in release announcements. Worth the five-minute rewrite before merge.

Panel summary

Persona	B	R	N	Takeaway
Python architect	0	0	0	Schema failure -- see full findings.
CLI logging expert	0	0	0	No CLI output, logging, or diagnostic surface touched; upstream data fix removes spurious UPDATE. Ship.
DevX UX expert	0	0	0	Internal plan-logic fix eliminates a spurious non-converging UPDATE; no CLI surface changes. Ship.
Supply-chain security expert	0	0	1	Trust model is sound; only a comment overstates tag immutability.
OSS growth hacker	0	1	0	CHANGELOG entry buries the user-facing fix under resolver internals.
Doc writer	0	1	2	CHANGELOG entry is accurate but should focus on user-visible behavior.
Test coverage expert	0	0	0	Schema failure -- see full findings.
Performance expert	0	0	0	O(1) guard inside existing O(n) loop; no I/O, network, or algorithmic regression. Ship.

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

Top 2 follow-ups

1. [OSS growth hacker] Rewrite CHANGELOG entry for user-facing clarity: lead with symptom and corrected outcome, drop internal symbols. -- Release notes are a conversion surface; maintainer-centric jargon reduces shareability and user confidence.
2. [Supply-chain security expert] Rephrase the comment at src/apm_cli/install/plan.py:225 to reference lockfile-pinned SHA trust rather than claiming tags are immutable. -- Accurate comments prevent future contributors from making incorrect assumptions about git tag mutability.

Recommendation

Merge after tightening the CHANGELOG entry to a user-facing symptom/outcome statement and rephrasing the tag-immutability comment. Both are small in-PR edits. No blocking findings from any validated panelist; the fix is correct, scoped, and tested.

---

Full per-persona findings

Python architect

No findings. Panelist response remained schema-invalid after two retries; the schema failure was surfaced to CEO arbitration.

CLI logging expert

No findings.

DevX UX expert

No findings.

Supply-chain security expert

• [nit] Comment calls git tags "immutable" but git tags are force-pushable at src/apm_cli/install/plan.py:225
  The code is sound because the locked commit SHA is the trust anchor; the comment overstates tag immutability.
  Suggested: Replace "immutable tag" with "tag-pinned lockfile entry" and identify the locked SHA as integrity anchor.

OSS growth hacker

• [recommended] CHANGELOG entry is maintainer-centric; rewrite for user-facing clarity at CHANGELOG.md:10
  The entry leads with resolver mechanics and internal symbols instead of the symptom and corrected outcome.
  Suggested: State that apm update now converges for git-source semver deps already at their locked tag.

Auth expert -- inactive

PR touches only CHANGELOG.md, src/apm_cli/install/plan.py, and tests/unit/install/test_plan.py -- no auth, token, credential, or host-resolution surface.

Doc writer

• [recommended] CHANGELOG entry exposes internal Python symbols that belong in the commit message, not user-facing release notes at CHANGELOG.md
  The entry names resolved_reference, build_update_plan, and None rather than just user-visible behavior.
  Suggested: Use a compact symptom/outcome statement and say branch dependencies remain unaffected.
• [nit] Cross-reference to fix(registry): registry deps as first-class citizens in display/UX surfaces #1908 is unexplained jargon for changelog readers at CHANGELOG.md
  CHANGELOG entries should be self-contained.
  Suggested: Drop fix(registry): registry deps as first-class citizens in display/UX surfaces #1908.
• [nit] Double-hyphen separator is less readable than a sentence break at CHANGELOG.md
  Use a direct sentence for readability.

Test coverage expert

No findings. Panelist response remained schema-invalid after two retries; the schema failure and its unvalidated integration concern were surfaced to CEO arbitration.

Performance expert

No findings.

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

[danielmeppiel]

APM Review Panel: ship_now

Fix git-semver apm update idempotency: the guarded plan fallback borrows the locked SHA only when no fresh resolution exists, eliminating non-converging update loops.

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

Five of nine panelists returned schema failures (python-architect, cli-logging-expert, supply-chain-security-expert, auth-expert, test-coverage-expert). These were output-shape failures, not substantive objections. Independent deterministic evidence fills the gap: two mutation-proven unit tests confirm the guard logic, 3024 relevant tests passed after merging current main, canonical lint is clean, and all GitHub checks are green.

The four panelists with valid returns converge on ship. DevX confirms the fix removes a confusing non-converging update loop. Doc-writer confirms the CHANGELOG is accurate and existing docs remain consistent. Performance confirms the PR guard is O(1) inside an existing O(n) loop with no I/O; its two findings target files already in current main and are outside this PR. Growth offers optional wording polish, while DevX and docs independently find the rewritten entry clear. No validated in-scope follow-up remains.

Dissent. No substantive dissent. Five schema failures prevented validated opinions from those personas; independent CI and mutation evidence covers the load-bearing correctness claims.

Aligned with: Portable by manifest: Lockfile convergence reaches a stable state regardless of environment, strengthening cross-machine portability. Pragmatic as npm: Idempotent update is table-stakes package-manager behavior; this fix closes that gap.

Panel summary

Persona	B	R	N	Takeaway
Python architect	0	0	0	Schema failure -- see full findings.
CLI logging expert	0	0	0	Schema failure -- see full findings.
DevX UX expert	0	0	0	Pure internal logic fix removes the non-converging update loop; no UX concerns.
Supply-chain security expert	0	0	0	Schema failure -- see full findings.
OSS growth hacker	0	0	1	CHANGELOG credits the author and explains user-visible convergence; ship.
Auth expert	0	0	0	Schema failure -- see full findings.
Doc writer	0	0	0	CHANGELOG and existing update/lockfile docs remain consistent.
Test coverage expert	0	0	0	Schema failure -- see full findings.
Performance expert	0	0	2	PR guard is O(1) inside existing O(n) work with no I/O; ship.

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

Recommendation

Merge as-is. The fix is narrowly scoped, mutation-proven, CI-green, and unopposed by every panelist that returned a valid response. No follow-ups to track.

Folded in this run

• (panel) Rewrote the CHANGELOG entry around user-visible convergence and preserved external-author credit -- resolved in 6d7d0eb142d753bc4e81aa75f7cd91e0f73d0dda.
• (panel) Rephrased code and test comments to identify the locked SHA as the integrity anchor without treating git tags as immutable -- resolved in 6d7d0eb142d753bc4e81aa75f7cd91e0f73d0dda.

Copilot signals reviewed

• Two fetch rounds completed; Copilot produced no inline review comments to classify.

Regression-trap evidence (mutation-break gate)

• TestBuildUpdatePlan::test_git_semver_tag_dep_unchanged_when_cached_and_ref_matches -- deleted the locked-commit fallback; test FAILED as expected; guard restored.
• TestBuildUpdatePlan::test_git_branch_dep_tip_advance_still_shows_update -- removed the fresh-SHA and tag-only guard conditions; 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/ completed with no diagnostics. YAML I/O, file length, relative_to, pylint R0801, auth-signal, and architecture-boundary guards also passed.

CI

All 13 checks on 7e32f66dbea525d7c45185435d40e1a3eefe6931 passed, including both Linux test shards, coverage combine, Lint, CodeQL, binary smoke, spec conformance, NOTICE drift, Merge Gate, APM Self-Check, and CLA. CI fix iterations: 0. Primary run: https://github.com/microsoft/apm/actions/runs/29197385197

Mergeability status

Captured from gh pr view 2165 --repo microsoft/apm --json number,headRefOid,mergeable,mergeStateStatus,statusCheckRollup after the final push.

PR	head SHA	CEO stance	iters	folds	defers	Copilot rounds	CI	mergeable	mergeStateStatus	notes
#2165	7e32f66	ship_now	1	2	0	2	green	MERGEABLE	BLOCKED	pending required review

Convergence

1 outer iteration; 2 Copilot rounds. Final panel stance: ship_now.

Ready for maintainer review.

---

Full per-persona findings

Python architect

No validated findings. The response remained outside the required JSON-only envelope after two retries.

CLI logging expert

No validated findings. The response remained on an obsolete schema or outside the JSON-only envelope after two retries.

DevX UX expert

No findings.

Supply-chain security expert

No validated findings. The response remained outside the required JSON-only envelope after two retries.

OSS growth hacker

• [nit] CHANGELOG entry could use a more immediately quotable user hook at CHANGELOG.md:12
  The entry is accurate; the suggested wording is optional polish. Doc-writer and DevX both found the current user-facing rewrite clear.

Auth expert

No validated findings. The conditional reviewer repeatedly returned an obsolete schema; the touched files are auth-neutral.

Doc writer

No findings.

Test coverage expert

No validated findings. The response remained outside the required JSON-only envelope after two retries. Mutation-break evidence and green CI independently verify the changed behavior.

Performance expert

• [nit] Out-of-scope main change: update recheck guard is ordered for fast exit at src/apm_cli/deps/apm_resolver.py:972
  This file change is already in current main, not PR fix(deps): git-semver update idempotency in build_update_plan #2165. No action for this PR.
• [nit] Out-of-scope main change: local dependency mapping remains linear at src/apm_cli/commands/deps/cli.py:287
  This file change is already in current main, not PR fix(deps): git-semver update idempotency in build_update_plan #2165. No action for this PR.

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