fix: honor explicit global install refs (closes #1555) (#1559)

* fix: honor explicit install refs for existing deps

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

* fix: clarify install ref update feedback

Fold panel follow-ups for the explicit-ref install fix by making validation output distinguish an updated manifest entry from a no-op, using generic manifest persistence wording, and documenting the fix in the changelog.

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

---------

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

Author: danielmeppiel

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- `apm install -g <package>#<ref>` now updates an existing unpinned global dependency entry instead of leaving the manifest floating. (#1559)
 - `apm install` now honors manifest `targets:` without falling back to the legacy Copilot target when singular `target:` is absent. (#1560)
 
 ## [0.16.0] - 2026-05-28

src/apm_cli/commands/install.py

@@ -48,9 +48,9 @@
 from apm_cli.install.package_resolution import (
     GIT_PARENT_USER_SCOPE_ERROR,
     dependency_reference_to_yaml_entry,
-    merge_structured_entry_into_current_deps,
     persist_dependency_list_if_changed,
     resolve_parsed_dependency_reference,
+    update_existing_dependency_entry_if_needed,
     user_scope_rejection_reason,
 )
 
@@ -536,24 +536,24 @@ def warning_handler(msg):
             logger=logger,
             dep_ref=dep_ref,
         ):
+            updates_existing_entry = update_existing_dependency_entry_if_needed(
+                current_deps,
+                already_in_deps=already_in_deps,
+                apm_yml_entries=_apm_yml_entries,
+                canonical=canonical,
+                dep_ref=dep_ref,
+                identity=identity,
+                dependency_reference_cls=DependencyReference,
+                logger=logger,
+            )
             valid_outcomes.append((canonical, already_in_deps))
             if logger:
-                logger.validation_pass(canonical, already_present=already_in_deps)
+                logger.validation_pass(canonical, already_in_deps, updates_existing_entry)
 
             if not already_in_deps:
                 validated_packages.append(canonical)
                 existing_identities.add(identity)  # prevent duplicates within batch
-            elif canonical in _apm_yml_entries:
-                structured_entry = _apm_yml_entries[canonical]
-                merge_structured_entry_into_current_deps(
-                    current_deps,
-                    structured_entry,
-                    identity,
-                    canonical,
-                    dependency_reference_cls=DependencyReference,
-                    logger=logger,
-                )
-                dependencies_changed = True
+            dependencies_changed = dependencies_changed or updates_existing_entry
             if marketplace_provenance:
                 _marketplace_provenance[identity] = marketplace_provenance
         else:

src/apm_cli/core/command_logger.py

@@ -218,9 +218,11 @@ def validation_start(self, count: int):
         noun = "package" if count == 1 else "packages"
         _rich_info(f"Validating {count} {noun}...", symbol="gear")
 
-    def validation_pass(self, canonical: str, already_present: bool):
+    def validation_pass(self, canonical: str, already_present: bool, updated: bool = False):
         """Log a package that passed validation."""
-        if already_present:
+        if updated:
+            _rich_echo(f"{canonical} (updated ref in apm.yml)", color="dim", symbol="check")
+        elif already_present:
             _rich_echo(f"{canonical} (already in apm.yml)", color="dim", symbol="check")
         else:
             _rich_success(canonical, symbol="check")

src/apm_cli/install/package_resolution.py

@@ -129,6 +129,65 @@ def user_scope_rejection_reason(dep_ref: Any, scope: Any) -> str | None:
     return None
 
 
+def manifest_has_different_entry_for_identity(
+    current_deps: builtins.list,
+    identity: str,
+    canonical: str,
+    *,
+    dependency_reference_cls: Any,
+) -> bool:
+    """Return True when apm.yml already has *identity* but not *canonical*."""
+    for dep_entry in current_deps:
+        try:
+            if isinstance(dep_entry, builtins.str):
+                existing_ref = dependency_reference_cls.parse(dep_entry)
+            elif isinstance(dep_entry, builtins.dict):
+                existing_ref = dependency_reference_cls.parse_from_dict(dep_entry)
+            else:
+                continue
+        except (ValueError, TypeError, AttributeError, KeyError):
+            continue
+        if existing_ref.get_identity() == identity:
+            return existing_ref.to_canonical() != canonical
+    return False
+
+
+def update_existing_dependency_entry_if_needed(
+    current_deps: builtins.list,
+    *,
+    already_in_deps: bool,
+    apm_yml_entries: dict,
+    canonical: str,
+    dep_ref: Any,
+    identity: str,
+    dependency_reference_cls: Any,
+    logger: Any = None,
+) -> bool:
+    """Rewrite an existing manifest dep when the requested ref changed."""
+    should_update = already_in_deps and (
+        canonical in apm_yml_entries
+        or (
+            dep_ref.reference
+            and manifest_has_different_entry_for_identity(
+                current_deps,
+                identity,
+                canonical,
+                dependency_reference_cls=dependency_reference_cls,
+            )
+        )
+    )
+    if should_update:
+        merge_structured_entry_into_current_deps(
+            current_deps,
+            apm_yml_entries.get(canonical, dep_ref.to_apm_yml_entry()),
+            identity,
+            canonical,
+            dependency_reference_cls=dependency_reference_cls,
+            logger=logger,
+        )
+    return should_update
+
+
 def merge_structured_entry_into_current_deps(
     current_deps: builtins.list,
     structured_entry: dict,
@@ -183,7 +242,7 @@ def persist_dependency_list_if_changed(
 
         dump_yaml(data, apm_yml_path)
         if logger:
-            logger.success(f"Updated {apm_yml_filename} to preserve marketplace subdirectory entry")
+            logger.success(f"Updated {apm_yml_filename} dependency entries")
     except Exception as e:
         if logger:
             logger.error(f"Failed to write {apm_yml_filename}: {e}")

tests/unit/commands/test_install_resolve_refs.py

@@ -15,7 +15,7 @@
 from unittest.mock import MagicMock, patch
 
 # The function under test lives in the commands module.
-from apm_cli.commands.install import _resolve_package_references
+from apm_cli.commands.install import _check_package_conflicts, _resolve_package_references
 from apm_cli.models.apm_package import APMPackage, clear_apm_yml_cache
 from apm_cli.models.dependency.reference import DependencyReference
 
@@ -165,6 +165,24 @@ def test_mixed_new_and_preexisting(self, mock_dep_cls, mock_validate):
         assert "github.com/owner/new-pkg" in existing
         assert len(existing) == 2
 
+    @patch("apm_cli.commands.install._validate_package_exists", return_value=True)
+    def test_existing_unpinned_dependency_is_updated_when_cli_supplies_ref(self, mock_validate):
+        """An explicit CLI ref for an existing dep must replace the unpinned manifest entry."""
+        current_deps = ["danielmeppiel/genesis"]
+        existing = _check_package_conflicts(current_deps)
+
+        valid, invalid, validated, _mkt, _entries, changed = _resolve_package_references(
+            ["danielmeppiel/genesis#v0.4.0"],
+            current_deps,
+            existing,
+        )
+
+        assert invalid == []
+        assert valid == [("danielmeppiel/genesis#v0.4.0", True)]
+        assert validated == []
+        assert changed is True
+        assert current_deps == ["danielmeppiel/genesis#v0.4.0"]
+
 
 class TestResolvePackageReferencesInvalidInput:
     """Invalid packages must not mutate the identity set."""

tests/unit/install/test_package_resolution_persistence.py

@@ -0,0 +1,30 @@
+"""Tests for dependency-list persistence messaging."""
+
+from __future__ import annotations
+
+from unittest.mock import Mock, patch
+
+from apm_cli.install.package_resolution import persist_dependency_list_if_changed
+
+
+def test_persist_dependency_list_reports_generic_manifest_update():
+    """Manifest rewrites should not claim every change is marketplace-specific."""
+    logger = Mock()
+    data = {"dependencies": {"apm": []}}
+    current_deps = ["danielmeppiel/genesis#v0.4.0"]
+
+    with patch("apm_cli.utils.yaml_io.dump_yaml") as dump_yaml:
+        persist_dependency_list_if_changed(
+            dependencies_changed=True,
+            data=data,
+            dep_section="dependencies",
+            current_deps=current_deps,
+            apm_yml_path="apm.yml",
+            apm_yml_filename="apm.yml",
+            logger=logger,
+            rich_error=Mock(),
+            sys_exit=Mock(),
+        )
+
+    dump_yaml.assert_called_once_with(data, "apm.yml")
+    logger.success.assert_called_once_with("Updated apm.yml dependency entries")

tests/unit/test_command_logger.py

@@ -197,6 +197,12 @@ def test_validation_pass_existing(self, mock_echo):
         logger.validation_pass("microsoft/repo", already_present=True)
         assert "already in apm.yml" in mock_echo.call_args[0][0]
 
+    @patch("apm_cli.core.command_logger._rich_echo")
+    def test_validation_pass_existing_updated(self, mock_echo):
+        logger = InstallLogger()
+        logger.validation_pass("microsoft/repo#v1", already_present=True, updated=True)
+        assert "updated ref in apm.yml" in mock_echo.call_args[0][0]
+
     @patch("apm_cli.core.command_logger._rich_error")
     def test_validation_fail(self, mock_error):
         logger = InstallLogger()