Fix transitive dependency handling in compile and orphan detection

docs/enhanced-primitive-discovery.md

@@ -116,7 +116,7 @@ if collection.has_conflicts():
 
 ### Dependency Declaration Order
 
-The system reads `apm.yml` to determine the order in which dependencies should be processed:
+The system reads `apm.yml` to determine the order in which direct dependencies should be processed. Transitive dependencies (resolved automatically via dependency chains) are read from `apm.lock` and appended after direct dependencies:
 
 ```yaml
 # apm.yml
@@ -129,7 +129,7 @@ dependencies:
     - user/utilities
 ```
 
-Dependencies are processed in this exact order. If multiple dependencies provide primitives with the same name, the first one declared wins.
+Direct dependencies are processed first, in declaration order. Transitive dependencies from `apm.lock` are appended after. If multiple dependencies provide primitives with the same name, the first one declared wins.
 
 ## Directory Structure
 

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 LockFile
     from apm_cli.models.apm_package import APMPackage, DependencyReference
     from apm_cli.integration import PromptIntegrator, AgentIntegrator
 
@@ -130,14 +131,16 @@ def _lazy_confirm():
 
 
 def _check_orphaned_packages():
-    """Check for packages in apm_modules/ that are not declared in apm.yml.
+    """Check for packages in apm_modules/ that are not declared in apm.yml or apm.lock.
+
+    Considers both direct dependencies (from apm.yml) and transitive dependencies
+    (from apm.lock) as expected packages, so transitive deps are not falsely
+    flagged as orphaned.
 
     Returns:
         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 []
@@ -164,6 +167,10 @@ def _check_orphaned_packages():
                 except ValueError:
                     # If path is not relative to apm_modules_dir, use as-is
                     expected_installed.add(str(install_path))
+
+            # Also include transitive dependencies from apm.lock
+            lockfile_paths = LockFile.installed_paths_for_project(Path.cwd())
+            expected_installed.update(lockfile_paths)
         except Exception:
             return []  # If can't parse apm.yml, assume no orphans
 
@@ -194,7 +201,7 @@ def _check_orphaned_packages():
                                         path_key = f"{level1_dir.name}/{level2_dir.name}/{level3_dir.name}"
                                         installed_packages.append(path_key)
 
-        # Find orphaned packages (installed but not declared)
+        # Find orphaned packages (installed but not declared or locked)
         orphaned_packages = []
         for org_repo_name in installed_packages:
             if org_repo_name not in expected_installed:
@@ -789,6 +796,10 @@ def prune(ctx, dry_run):
                         )
                     elif len(repo_parts) >= 2:
                         expected_installed.add(f"{repo_parts[0]}/{repo_parts[1]}")
+
+            # Also include transitive dependencies from apm.lock
+            lockfile_paths = LockFile.installed_paths_for_project(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/commands/deps.py

@@ -29,7 +29,9 @@ def list_packages():
         # Import Rich components with fallback
         from rich.table import Table
         from rich.console import Console
-        console = Console()
+        import shutil
+        term_width = shutil.get_terminal_size((120, 24)).columns
+        console = Console(width=max(120, term_width))
         has_rich = True
     except ImportError:
         has_rich = False

src/apm_cli/deps/__init__.py

@@ -29,4 +29,5 @@
     'LockFile',
     'LockedDependency',
     'get_lockfile_path',
+
 ]

src/apm_cli/deps/lockfile.py

@@ -200,11 +200,72 @@ 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)
 
+    @classmethod
+    def installed_paths_for_project(cls, project_root: Path) -> List[str]:
+        """Load apm.lock from project_root and return installed paths.
+
+        Returns an empty list if the lockfile is missing, corrupt, or
+        unreadable.
+
+        Args:
+            project_root: Path to project root containing apm.lock.
+
+        Returns:
+            List[str]: Relative installed paths (e.g., ['owner/repo']),
+                       ordered by depth then repo_url (no duplicates).
+        """
+        try:
+            lockfile = cls.read(project_root / "apm.lock")
+            if not lockfile:
+                return []
+            return lockfile.get_installed_paths(project_root / "apm_modules")
+        except (FileNotFoundError, yaml.YAMLError, ValueError, KeyError):
+            return []
+
 
 def get_lockfile_path(project_root: Path) -> Path:
     """Get the path to the lock file for a project."""
     return project_root / "apm.lock"
+
+
+def get_lockfile_installed_paths(project_root: Path) -> List[str]:
+    """Deprecated: use LockFile.installed_paths_for_project() instead."""
+    return LockFile.installed_paths_for_project(project_root)

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 LockFile
 
 
 # Common primitive patterns for local discovery (with recursive search)
@@ -183,9 +184,15 @@ def scan_dependency_primitives(base_dir: str, collection: PrimitiveCollection) -
 
 
 def get_dependency_declaration_order(base_dir: str) -> List[str]:
-    """Get APM dependency installed paths in their declaration order from apm.yml.
+    """Get APM dependency installed paths in their declaration order.
 
-    Returns the actual installed paths for each dependency, which differs for:
+    The returned list contains the actual installed path for each dependency,
+    combining:
+    1. Direct dependencies from apm.yml (highest priority, declaration order)
+    2. Transitive dependencies from apm.lock (appended after direct deps)
+
+    This ensures transitive dependencies are included in primitive discovery
+    and compilation, not just direct dependencies. The installed path differs for:
     - Regular packages: owner/repo (GitHub) or org/project/repo (ADO)
     - Virtual packages: owner/virtual-pkg-name (GitHub) or org/project/virtual-pkg-name (ADO)
 
@@ -243,6 +250,14 @@ def get_dependency_declaration_order(base_dir: str) -> List[str]:
                 # This matches our org-namespaced directory structure
                 dependency_names.append(dep.repo_url)
 
+        # Include transitive dependencies from apm.lock
+        # Direct deps from apm.yml have priority; transitive deps are appended
+        lockfile_paths = LockFile.installed_paths_for_project(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
 
     except Exception as e:

tests/integration/test_skill_install.py

@@ -105,7 +105,7 @@ def test_install_skill_updates_apm_yml(self, temp_project, apm_command):
         assert "anthropics/skills/skills/brand-guidelines" in content
 
     def test_skill_detection_in_output(self, temp_project, apm_command):
-        """Verify CLI output shows 'Claude Skill (SKILL.md detected)'."""
+        """Verify CLI output shows skill integration message."""
         result = subprocess.run(
             [apm_command, "install", "anthropics/skills/skills/brand-guidelines", "--verbose"],
             cwd=temp_project,
@@ -114,8 +114,8 @@ def test_skill_detection_in_output(self, temp_project, apm_command):
             timeout=120
         )
 
-        # Check for skill detection message
-        assert "Claude Skill" in result.stdout or "SKILL.md detected" in result.stdout
+        # Check for skill detection/integration message
+        assert "Skill integrated" in result.stdout or "Claude Skill" in result.stdout or "SKILL.md detected" in result.stdout
 
 
 class TestClaudeSkillWithResources:

tests/test_enhanced_discovery.py

@@ -412,6 +412,116 @@ def test_collection_methods(self):
         chatmode_conflicts = collection.get_conflicts_by_type("chatmode")
         self.assertEqual(len(chatmode_conflicts), 0)  # No conflicts yet
 
+    def test_dependency_order_includes_transitive_from_lockfile(self):
+        """Test that transitive dependencies from apm.lock are included in declaration order."""
+        from apm_cli.deps.lockfile import LockFile, LockedDependency
+
+        # Create apm.yml with only one direct dependency
+        dependencies = {"apm": ["rieraj/team-cot-agent-instructions"]}
+        self._create_apm_yml(dependencies)
+
+        # Create apm.lock with transitive dependencies
+        lockfile = LockFile()
+        lockfile.add_dependency(LockedDependency(
+            repo_url="rieraj/team-cot-agent-instructions",
+            depth=1,
+        ))
+        lockfile.add_dependency(LockedDependency(
+            repo_url="rieraj/division-ime-agent-instructions",
+            depth=2,
+            resolved_by="rieraj/team-cot-agent-instructions",
+        ))
+        lockfile.add_dependency(LockedDependency(
+            repo_url="rieraj/autodesk-agent-instructions",
+            depth=3,
+            resolved_by="rieraj/division-ime-agent-instructions",
+        ))
+        lockfile.write(self.temp_dir_path / "apm.lock")
+
+        order = get_dependency_declaration_order(str(self.temp_dir_path))
+
+        # Direct dep should come first, then transitive deps from lockfile
+        self.assertIn("rieraj/team-cot-agent-instructions", order)
+        self.assertIn("rieraj/division-ime-agent-instructions", order)
+        self.assertIn("rieraj/autodesk-agent-instructions", order)
+        # Direct dep first
+        self.assertEqual(order[0], "rieraj/team-cot-agent-instructions")
+        self.assertEqual(len(order), 3)
+
+    def test_dependency_order_no_lockfile(self):
+        """Test that dependency order works without a lockfile (backward compat)."""
+        # Create apm.yml with dependencies but no lockfile
+        dependencies = {"apm": ["company/standards", "team/workflows"]}
+        self._create_apm_yml(dependencies)
+
+        order = get_dependency_declaration_order(str(self.temp_dir_path))
+        self.assertEqual(order, ["company/standards", "team/workflows"])
+
+    def test_dependency_order_lockfile_no_duplicates(self):
+        """Test that direct deps already in apm.yml are not duplicated from lockfile."""
+        from apm_cli.deps.lockfile import LockFile, LockedDependency
+
+        # Create apm.yml with all deps listed directly
+        dependencies = {"apm": [
+            "rieraj/team-cot",
+            "rieraj/division-ime",
+            "rieraj/autodesk",
+        ]}
+        self._create_apm_yml(dependencies)
+
+        # Create apm.lock that also contains all deps
+        lockfile = LockFile()
+        lockfile.add_dependency(LockedDependency(repo_url="rieraj/team-cot", depth=1))
+        lockfile.add_dependency(LockedDependency(repo_url="rieraj/division-ime", depth=1))
+        lockfile.add_dependency(LockedDependency(repo_url="rieraj/autodesk", depth=1))
+        lockfile.write(self.temp_dir_path / "apm.lock")
+
+        order = get_dependency_declaration_order(str(self.temp_dir_path))
+        # No duplicates
+        self.assertEqual(len(order), 3)
+        self.assertEqual(len(set(order)), 3)
+
+    def test_scan_dependency_primitives_with_transitive(self):
+        """Test that scan_dependency_primitives finds transitive dep primitives."""
+        from apm_cli.deps.lockfile import LockFile, LockedDependency
+
+        # Create apm.yml with only one direct dependency
+        dependencies = {"apm": ["owner/direct-dep"]}
+        self._create_apm_yml(dependencies)
+
+        # Create apm.lock with a transitive dependency
+        lockfile = LockFile()
+        lockfile.add_dependency(LockedDependency(repo_url="owner/direct-dep", depth=1))
+        lockfile.add_dependency(LockedDependency(
+            repo_url="owner/transitive-dep", depth=2,
+            resolved_by="owner/direct-dep",
+        ))
+        lockfile.write(self.temp_dir_path / "apm.lock")
+
+        # Create dependency directories with primitives
+        direct_dep_dir = self.temp_dir_path / "apm_modules" / "owner" / "direct-dep" / ".apm" / "instructions"
+        direct_dep_dir.mkdir(parents=True, exist_ok=True)
+        self._create_primitive_file(
+            direct_dep_dir / "direct.instructions.md",
+            "instruction", "direct"
+        )
+
+        transitive_dep_dir = self.temp_dir_path / "apm_modules" / "owner" / "transitive-dep" / ".apm" / "instructions"
+        transitive_dep_dir.mkdir(parents=True, exist_ok=True)
+        self._create_primitive_file(
+            transitive_dep_dir / "transitive.instructions.md",
+            "instruction", "transitive"
+        )
+
+        collection = PrimitiveCollection()
+        scan_dependency_primitives(str(self.temp_dir_path), collection)
+
+        # Both direct and transitive deps should be found
+        self.assertEqual(len(collection.instructions), 2)
+        instruction_names = {i.name for i in collection.instructions}
+        self.assertIn("direct", instruction_names)
+        self.assertIn("transitive", instruction_names)
+
 
 if __name__ == '__main__':
     unittest.main()