refactor: address PR review comments for transitive deps fix

- Move get_lockfile_installed_paths logic into LockFile.get_installed_paths()
  method (foutoucour review)
- Use .as_posix() for Windows-safe path normalization (copilot-reviewer)
- Narrow except Exception to specific error types (copilot-reviewer)
- Move inline imports to top-level in discovery.py and cli.py (foutoucour)
- Remove unused imports in test file (tempfile, pytest)

Author: danielmeppiel

src/apm_cli/cli.py

@@ -41,6 +41,7 @@
 try:
     from apm_cli.deps.apm_resolver import APMDependencyResolver
     from apm_cli.deps.github_downloader import GitHubPackageDownloader
+    from apm_cli.deps.lockfile import get_lockfile_installed_paths
     from apm_cli.models.apm_package import APMPackage, DependencyReference
     from apm_cli.integration import PromptIntegrator, AgentIntegrator
 
@@ -140,8 +141,6 @@ def _check_orphaned_packages():
         List[str]: List of orphaned package names in org/repo or org/project/repo format
     """
     try:
-        from pathlib import Path
-
         # Check if apm.yml exists
         if not Path("apm.yml").exists():
             return []
@@ -170,12 +169,8 @@ def _check_orphaned_packages():
                     expected_installed.add(str(install_path))
 
             # Also include transitive dependencies from apm.lock
-            try:
-                from apm_cli.deps.lockfile import get_lockfile_installed_paths
-                lockfile_paths = get_lockfile_installed_paths(Path.cwd())
-                expected_installed.update(lockfile_paths)
-            except ImportError:
-                pass  # Lockfile module not available
+            lockfile_paths = get_lockfile_installed_paths(Path.cwd())
+            expected_installed.update(lockfile_paths)
         except Exception:
             return []  # If can't parse apm.yml, assume no orphans
 
@@ -803,12 +798,8 @@ def prune(ctx, dry_run):
                         expected_installed.add(f"{repo_parts[0]}/{repo_parts[1]}")
 
             # Also include transitive dependencies from apm.lock
-            try:
-                from apm_cli.deps.lockfile import get_lockfile_installed_paths
-                lockfile_paths = get_lockfile_installed_paths(Path.cwd())
-                expected_installed.update(lockfile_paths)
-            except ImportError:
-                pass  # Lockfile module not available
+            lockfile_paths = get_lockfile_installed_paths(Path.cwd())
+            expected_installed.update(lockfile_paths)
         except Exception as e:
             _rich_error(f"Failed to parse apm.yml: {e}")
             sys.exit(1)

src/apm_cli/deps/lockfile.py

@@ -200,6 +200,40 @@ def from_installed_packages(
 
         return lock
 
+    def get_installed_paths(self, apm_modules_dir: Path) -> List[str]:
+        """Get relative installed paths for all dependencies in this lockfile.
+
+        Computes expected installed paths for all dependencies, including
+        transitive ones. Used by:
+        - Primitive discovery to find all dependency primitives
+        - Orphan detection to avoid false positives for transitive deps
+
+        Args:
+            apm_modules_dir: Path to the apm_modules directory.
+
+        Returns:
+            List[str]: POSIX-style relative installed paths (e.g., ['owner/repo']),
+                       ordered by depth then repo_url (no duplicates).
+        """
+        seen: set = set()
+        paths: List[str] = []
+        for dep in self.get_all_dependencies():
+            dep_ref = DependencyReference(
+                repo_url=dep.repo_url,
+                host=dep.host,
+                virtual_path=dep.virtual_path,
+                is_virtual=dep.is_virtual,
+            )
+            install_path = dep_ref.get_install_path(apm_modules_dir)
+            try:
+                rel_path = install_path.relative_to(apm_modules_dir).as_posix()
+            except ValueError:
+                rel_path = Path(install_path).as_posix()
+            if rel_path not in seen:
+                seen.add(rel_path)
+                paths.append(rel_path)
+        return paths
+
     def save(self, path: Path) -> None:
         """Save lock file to disk (alias for write)."""
         self.write(path)
@@ -213,12 +247,9 @@ def get_lockfile_path(project_root: Path) -> Path:
 def get_lockfile_installed_paths(project_root: Path) -> List[str]:
     """Get installed paths for all dependencies recorded in apm.lock.
 
-    Reads the lockfile and computes expected installed paths for all
-    dependencies, including transitive ones. Used by:
-    - Primitive discovery to find all dependency primitives
-    - Orphan detection to avoid false positives for transitive deps
-
-    Returns an empty list if the lockfile is missing, corrupt, or unreadable.
+    Convenience wrapper that loads the lockfile and delegates to
+    LockFile.get_installed_paths(). Returns an empty list if the
+    lockfile is missing, corrupt, or unreadable.
 
     Args:
         project_root: Path to project root containing apm.lock.
@@ -232,25 +263,6 @@ def get_lockfile_installed_paths(project_root: Path) -> List[str]:
         lockfile = LockFile.read(lockfile_path)
         if not lockfile:
             return []
-
-        apm_modules_dir = project_root / "apm_modules"
-        paths: List[str] = []
-        seen: set = set()
-        for dep in lockfile.get_all_dependencies():
-            dep_ref = DependencyReference(
-                repo_url=dep.repo_url,
-                host=dep.host,
-                virtual_path=dep.virtual_path,
-                is_virtual=dep.is_virtual,
-            )
-            install_path = dep_ref.get_install_path(apm_modules_dir)
-            try:
-                rel_path = str(install_path.relative_to(apm_modules_dir))
-            except ValueError:
-                rel_path = str(install_path)
-            if rel_path not in seen:
-                seen.add(rel_path)
-                paths.append(rel_path)
-        return paths
-    except Exception:
+        return lockfile.get_installed_paths(project_root / "apm_modules")
+    except (FileNotFoundError, yaml.YAMLError, ValueError, KeyError):
         return []

src/apm_cli/primitives/discovery.py

@@ -8,6 +8,7 @@
 from .models import PrimitiveCollection
 from .parser import parse_primitive_file, parse_skill_file
 from ..models.apm_package import APMPackage
+from ..deps.lockfile import get_lockfile_installed_paths
 
 
 # Common primitive patterns for local discovery (with recursive search)
@@ -251,15 +252,11 @@ def get_dependency_declaration_order(base_dir: str) -> List[str]:
 
         # Include transitive dependencies from apm.lock
         # Direct deps from apm.yml have priority; transitive deps are appended
-        try:
-            from ..deps.lockfile import get_lockfile_installed_paths
-            lockfile_paths = get_lockfile_installed_paths(Path(base_dir))
-            direct_set = set(dependency_names)
-            for path in lockfile_paths:
-                if path not in direct_set:
-                    dependency_names.append(path)
-        except ImportError:
-            pass  # Lockfile module not available
+        lockfile_paths = get_lockfile_installed_paths(Path(base_dir))
+        direct_set = set(dependency_names)
+        for path in lockfile_paths:
+            if path not in direct_set:
+                dependency_names.append(path)
 
         return dependency_names
 

tests/unit/test_transitive_deps.py

@@ -6,10 +6,8 @@
 - get_dependency_declaration_order() includes transitive deps from lockfile
 """
 
-import tempfile
 from pathlib import Path
 
-import pytest
 import yaml
 
 from apm_cli.deps.lockfile import (