Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/enhanced-primitive-discovery.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
24 changes: 22 additions & 2 deletions src/apm_cli/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,11 @@ 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
Expand Down Expand Up @@ -164,6 +168,14 @@ 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
try:
from apm_cli.deps.lockfile import get_lockfile_installed_paths
Comment thread
danielmeppiel marked this conversation as resolved.
Outdated
lockfile_paths = get_lockfile_installed_paths(Path.cwd())
expected_installed.update(lockfile_paths)
except ImportError:
pass # Lockfile module not available
except Exception:
return [] # If can't parse apm.yml, assume no orphans

Expand Down Expand Up @@ -194,7 +206,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:
Expand Down Expand Up @@ -789,6 +801,14 @@ 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
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
except Exception as e:
_rich_error(f"Failed to parse apm.yml: {e}")
sys.exit(1)
Expand Down
3 changes: 2 additions & 1 deletion src/apm_cli/deps/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from .verifier import verify_dependencies, install_missing_dependencies, load_apm_config
from .github_downloader import GitHubPackageDownloader
from .package_validator import PackageValidator
from .lockfile import LockFile, LockedDependency, get_lockfile_path
from .lockfile import LockFile, LockedDependency, get_lockfile_path, get_lockfile_installed_paths

__all__ = [
'sync_workflow_dependencies',
Expand All @@ -29,4 +29,5 @@
'LockFile',
'LockedDependency',
'get_lockfile_path',
'get_lockfile_installed_paths',
]
46 changes: 46 additions & 0 deletions src/apm_cli/deps/lockfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,3 +208,49 @@ def save(self, path: Path) -> None:
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]:
Comment thread
danielmeppiel marked this conversation as resolved.
"""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.

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_path = get_lockfile_path(project_root)
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)
Comment thread
danielmeppiel marked this conversation as resolved.
paths.append(rel_path)
return paths
Comment thread
danielmeppiel marked this conversation as resolved.
except Exception:
return []
Comment thread
danielmeppiel marked this conversation as resolved.
Outdated
21 changes: 20 additions & 1 deletion src/apm_cli/primitives/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,14 @@ 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, 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.

Returns the actual installed paths for each dependency, which differs for:
Comment thread
danielmeppiel marked this conversation as resolved.
Outdated
- Regular packages: owner/repo (GitHub) or org/project/repo (ADO)
Expand Down Expand Up @@ -243,6 +250,18 @@ 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
try:
from ..deps.lockfile import get_lockfile_installed_paths
Comment thread
danielmeppiel marked this conversation as resolved.
Outdated
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

return dependency_names

except Exception as e:
Expand Down
110 changes: 110 additions & 0 deletions tests/test_enhanced_discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Loading