Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
42 changes: 42 additions & 0 deletions docs/skills.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ APM copies skills directly to `.github/skills/` (primary) and `.claude/skills/`
| Package Type | Behavior |
|--------------|----------|
| **Has existing SKILL.md** | Entire skill folder copied to `.github/skills/{skill-name}/` |
| **Has sub-skills in `.apm/skills/`** | Each `.apm/skills/*/SKILL.md` also promoted to `.github/skills/{sub-skill-name}/` |
| **No SKILL.md and no primitives** | No skill folder created |

**Target Directories:**
Expand Down Expand Up @@ -227,6 +228,47 @@ apm install your-org/awesome-skills/skill-1
apm install your-org/awesome-skills/skill-2
```

### Option 4: Multi-skill Package

Bundle multiple skills inside a single APM package using `.apm/skills/`:

```
my-package/
β”œβ”€β”€ apm.yml
β”œβ”€β”€ SKILL.md # Parent skill (package-level guide)
└── .apm/
β”œβ”€β”€ instructions/
β”œβ”€β”€ prompts/
└── skills/
β”œβ”€β”€ skill-a/
β”‚ └── SKILL.md # Sub-skill A
└── skill-b/
└── SKILL.md # Sub-skill B
```

On install, APM promotes each sub-skill to a top-level `.github/skills/` entry alongside the parent β€” see [Sub-skill Promotion](#sub-skill-promotion) below.

### Sub-skill Promotion

When a package contains sub-skills in `.apm/skills/*/` subdirectories, APM promotes each to a top-level entry under `.github/skills/`. This ensures Copilot can discover them independently, since it only scans direct children of `.github/skills/`.

```
# Installed package with sub-skills:
apm_modules/org/repo/my-package/
β”œβ”€β”€ SKILL.md
└── .apm/
└── skills/
└── azure-naming/
└── SKILL.md

# Result after install:
.github/skills/
β”œβ”€β”€ my-package/ # Parent skill
β”‚ └── SKILL.md
└── azure-naming/ # Promoted sub-skill
└── SKILL.md
```

## Package Detection

APM automatically detects package types:
Expand Down
62 changes: 60 additions & 2 deletions src/apm_cli/integration/skill_integrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,40 @@ def _generate_skill_file(self, package_info, primitives: dict, skill_dir: Path)

return files_copied

@staticmethod
def _promote_sub_skills(sub_skills_dir: Path, target_skills_root: Path, parent_name: str, *, warn: bool = True) -> None:
"""Promote sub-skills from .apm/skills/ to top-level skill entries.

Args:
sub_skills_dir: Path to the .apm/skills/ directory in the source package.
target_skills_root: Root skills directory (e.g. .github/skills/ or .claude/skills/).
parent_name: Name of the parent skill (used in warning messages).
warn: Whether to emit a warning on name collisions.
"""
if not sub_skills_dir.is_dir():
return
for sub_skill_path in sub_skills_dir.iterdir():
if not sub_skill_path.is_dir():
continue
if not (sub_skill_path / "SKILL.md").exists():
continue
raw_sub_name = sub_skill_path.name
is_valid, _ = validate_skill_name(raw_sub_name)
sub_name = raw_sub_name if is_valid else normalize_skill_name(raw_sub_name)
target = target_skills_root / sub_name
if target.exists():
if warn:
try:
from apm_cli.cli import _rich_warning
_rich_warning(
f"Sub-skill '{sub_name}' from '{parent_name}' overwrites existing skill at {target_skills_root.name}/{sub_name}/"
)
except ImportError:
pass
shutil.rmtree(target)
target.mkdir(parents=True, exist_ok=True)
shutil.copytree(sub_skill_path, target, dirs_exist_ok=True)

def _integrate_native_skill(
self, package_info, project_root: Path, source_skill_md: Path
) -> SkillIntegrationResult:
Expand Down Expand Up @@ -811,10 +845,19 @@ def _integrate_native_skill(
shutil.rmtree(github_skill_dir)

github_skill_dir.parent.mkdir(parents=True, exist_ok=True)
shutil.copytree(package_path, github_skill_dir)
shutil.copytree(package_path, github_skill_dir,
ignore=shutil.ignore_patterns('.apm'))

files_copied = sum(1 for _ in github_skill_dir.rglob('*') if _.is_file())

# === Promote sub-skills to top-level entries ===
# Packages may contain sub-skills in .apm/skills/*/ subdirectories.
# Copilot only discovers .github/skills/<name>/SKILL.md (direct children),
# so we promote each sub-skill to an independent top-level entry.
sub_skills_dir = package_path / ".apm" / "skills"
github_skills_root = project_root / ".github" / "skills"
self._promote_sub_skills(sub_skills_dir, github_skills_root, skill_name, warn=True)

# === T7: Copy to .claude/skills/ (secondary - compatibility) ===
claude_dir = project_root / ".claude"
if claude_dir.exists() and claude_dir.is_dir():
Expand All @@ -824,7 +867,12 @@ def _integrate_native_skill(
shutil.rmtree(claude_skill_dir)

claude_skill_dir.parent.mkdir(parents=True, exist_ok=True)
shutil.copytree(package_path, claude_skill_dir)
shutil.copytree(package_path, claude_skill_dir,
ignore=shutil.ignore_patterns('.apm'))

# Promote sub-skills for Claude too
claude_skills_root = claude_dir / "skills"
self._promote_sub_skills(sub_skills_dir, claude_skills_root, skill_name, warn=False)

return SkillIntegrationResult(
skill_created=skill_created,
Expand Down Expand Up @@ -1031,6 +1079,16 @@ def sync_integration(self, apm_package, project_root: Path) -> Dict[str, int]:
is_valid, _ = validate_skill_name(raw_name)
skill_name = raw_name if is_valid else normalize_skill_name(raw_name)
installed_skill_names.add(skill_name)

# Also include promoted sub-skills from installed packages
install_path = dep.get_install_path(project_root / "apm_modules")
sub_skills_dir = install_path / ".apm" / "skills"
if sub_skills_dir.is_dir():
for sub_skill_path in sub_skills_dir.iterdir():
if sub_skill_path.is_dir() and (sub_skill_path / "SKILL.md").exists():
raw_sub = sub_skill_path.name
is_valid, _ = validate_skill_name(raw_sub)
installed_skill_names.add(raw_sub if is_valid else normalize_skill_name(raw_sub))

# Clean .github/skills/ (primary)
github_skills_dir = project_root / ".github" / "skills"
Expand Down
205 changes: 204 additions & 1 deletion tests/unit/integration/test_skill_integrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import tempfile
import shutil
from pathlib import Path
from unittest.mock import Mock
from unittest.mock import Mock, patch
from datetime import datetime

from apm_cli.integration.skill_integrator import SkillIntegrator, SkillIntegrationResult, to_hyphen_case, validate_skill_name, normalize_skill_name, copy_skill_to_target
Expand Down Expand Up @@ -2589,3 +2589,206 @@ def test_sync_aggregates_stats_from_both_locations(self):
assert result['files_removed'] == 2
assert not github_orphan.exists()
assert not claude_orphan.exists()


class TestSubSkillPromotion:
"""Test that sub-skills inside packages are promoted to top-level entries.

When a package contains .apm/skills/<sub-skill>/SKILL.md, each sub-skill
should be copied to .github/skills/<sub-skill>/ as an independent
top-level entry so Copilot can discover it.
"""

def setup_method(self):
self.temp_dir = tempfile.mkdtemp()
self.project_root = Path(self.temp_dir)
self.integrator = SkillIntegrator()

def teardown_method(self):
shutil.rmtree(self.temp_dir, ignore_errors=True)

def _create_package_info(
self,
name: str = "test-pkg",
install_path: Path = None,
package_type: PackageType = PackageType.CLAUDE_SKILL
) -> PackageInfo:
package = APMPackage(
name=name,
version="1.0.0",
package_path=install_path or self.project_root / "package",
source=f"github.com/test/{name}"
)
resolved_ref = ResolvedReference(
original_ref="main",
ref_type=GitReferenceType.BRANCH,
resolved_commit="abc123",
ref_name="main"
)
return PackageInfo(
package=package,
install_path=install_path or self.project_root / "package",
resolved_reference=resolved_ref,
installed_at=datetime.now().isoformat(),
package_type=package_type
)

def _create_package_with_sub_skills(self, name="parent-skill", sub_skills=None):
"""Create a package directory with a SKILL.md and sub-skills under .apm/skills/."""
package_dir = self.project_root / name
package_dir.mkdir()
(package_dir / "SKILL.md").write_text(f"---\nname: {name}\ndescription: Parent skill\n---\n# {name}\n")
if sub_skills:
skills_dir = package_dir / ".apm" / "skills"
skills_dir.mkdir(parents=True)
for sub_name in sub_skills:
sub_dir = skills_dir / sub_name
sub_dir.mkdir()
(sub_dir / "SKILL.md").write_text(
f"---\nname: {sub_name}\ndescription: Sub-skill {sub_name}\n---\n# {sub_name}\n"
)
return package_dir

def test_sub_skill_promoted_to_top_level(self):
"""Sub-skills under .apm/skills/ should be copied to .github/skills/ as top-level entries."""
package_dir = self._create_package_with_sub_skills(
"modernisation", sub_skills=["azure-naming"]
)
pkg_info = self._create_package_info(name="modernisation", install_path=package_dir)

self.integrator.integrate_package_skill(pkg_info, self.project_root)

# Parent skill exists
assert (self.project_root / ".github" / "skills" / "modernisation" / "SKILL.md").exists()
# .apm/ excluded from parent copy to avoid redundant storage
assert not (self.project_root / ".github" / "skills" / "modernisation" / ".apm").exists()
# Sub-skill promoted to top level
assert (self.project_root / ".github" / "skills" / "azure-naming" / "SKILL.md").exists()
content = (self.project_root / ".github" / "skills" / "azure-naming" / "SKILL.md").read_text()
assert "azure-naming" in content

def test_multiple_sub_skills_promoted(self):
"""All sub-skills in the package should be promoted."""
package_dir = self._create_package_with_sub_skills(
"my-package", sub_skills=["skill-a", "skill-b", "skill-c"]
)
pkg_info = self._create_package_info(name="my-package", install_path=package_dir)

self.integrator.integrate_package_skill(pkg_info, self.project_root)

for sub in ["skill-a", "skill-b", "skill-c"]:
assert (self.project_root / ".github" / "skills" / sub / "SKILL.md").exists()

def test_sub_skill_without_skill_md_not_promoted(self):
"""Directories under .apm/skills/ without SKILL.md should be ignored."""
package_dir = self._create_package_with_sub_skills("pkg", sub_skills=["valid-sub"])
# Add a directory without SKILL.md
(package_dir / ".apm" / "skills" / "no-skill-md").mkdir()
(package_dir / ".apm" / "skills" / "no-skill-md" / "README.md").write_text("# Not a skill")

pkg_info = self._create_package_info(name="pkg", install_path=package_dir)
self.integrator.integrate_package_skill(pkg_info, self.project_root)

assert (self.project_root / ".github" / "skills" / "valid-sub" / "SKILL.md").exists()
assert not (self.project_root / ".github" / "skills" / "no-skill-md").exists()

def test_sub_skill_name_collision_overwrites_with_warning(self):
"""If a promoted sub-skill name clashes with an existing skill, it overwrites and warns."""
# Pre-existing skill at top level
existing = self.project_root / ".github" / "skills" / "azure-naming"
existing.mkdir(parents=True)
(existing / "SKILL.md").write_text("# Old content")

package_dir = self._create_package_with_sub_skills(
"modernisation", sub_skills=["azure-naming"]
)
pkg_info = self._create_package_info(name="modernisation", install_path=package_dir)

with patch("apm_cli.cli._rich_warning") as mock_warning:
self.integrator.integrate_package_skill(pkg_info, self.project_root)

# Warning should have been emitted about the collision
mock_warning.assert_called_once()
assert "azure-naming" in mock_warning.call_args[0][0]
assert "modernisation" in mock_warning.call_args[0][0]

# Should be overwritten with sub-skill content
content = (self.project_root / ".github" / "skills" / "azure-naming" / "SKILL.md").read_text()
assert "Sub-skill azure-naming" in content
assert "Old content" not in content

def test_sub_skill_promoted_to_claude_skills(self):
"""Sub-skills should also be promoted under .claude/skills/ when .claude/ exists."""
(self.project_root / ".claude").mkdir()
package_dir = self._create_package_with_sub_skills(
"modernisation", sub_skills=["azure-naming"]
)
pkg_info = self._create_package_info(name="modernisation", install_path=package_dir)

self.integrator.integrate_package_skill(pkg_info, self.project_root)

assert (self.project_root / ".github" / "skills" / "azure-naming" / "SKILL.md").exists()
assert (self.project_root / ".claude" / "skills" / "azure-naming" / "SKILL.md").exists()

def test_sub_skill_name_normalization(self):
"""Sub-skills with invalid names should be normalized before promotion."""
package_dir = self.project_root / "my-package"
package_dir.mkdir()
(package_dir / "SKILL.md").write_text("---\nname: my-package\n---\n# Parent")
skills_dir = package_dir / ".apm" / "skills"
skills_dir.mkdir(parents=True)
# Create sub-skill with invalid name (uppercase + underscores)
bad_name_dir = skills_dir / "My_Azure_Skill"
bad_name_dir.mkdir()
(bad_name_dir / "SKILL.md").write_text("---\nname: My_Azure_Skill\n---\n# Bad name")

pkg_info = self._create_package_info(name="my-package", install_path=package_dir)
self.integrator.integrate_package_skill(pkg_info, self.project_root)

# Should be normalized to lowercase-hyphenated
assert not (self.project_root / ".github" / "skills" / "My_Azure_Skill").exists()
assert (self.project_root / ".github" / "skills" / "my-azure-skill" / "SKILL.md").exists()

def test_package_without_sub_skills_unchanged(self):
"""Packages without .apm/skills/ subdirectory should work as before."""
package_dir = self.project_root / "simple-skill"
package_dir.mkdir()
(package_dir / "SKILL.md").write_text("---\nname: simple-skill\n---\n# Simple")

pkg_info = self._create_package_info(name="simple-skill", install_path=package_dir)
result = self.integrator.integrate_package_skill(pkg_info, self.project_root)

assert result.skill_created is True
assert (self.project_root / ".github" / "skills" / "simple-skill" / "SKILL.md").exists()
skills = list((self.project_root / ".github" / "skills").iterdir())
assert len(skills) == 1

def test_sync_integration_preserves_promoted_sub_skills(self):
"""sync_integration should not orphan promoted sub-skills."""
# Set up installed package structure in apm_modules
apm_modules = self.project_root / "apm_modules"
owner_dir = apm_modules / "testorg" / "agent-library" / "modernisation"
owner_dir.mkdir(parents=True)
(owner_dir / "apm.yml").write_text("name: modernisation\nversion: 1.0.0\n")
(owner_dir / "SKILL.md").write_text("---\nname: modernisation\n---\n# Parent")
sub_dir = owner_dir / ".apm" / "skills" / "azure-naming"
sub_dir.mkdir(parents=True)
(sub_dir / "SKILL.md").write_text("---\nname: azure-naming\n---\n# Sub")

# Create the promoted skills in .github/skills/
for name in ["modernisation", "azure-naming"]:
d = self.project_root / ".github" / "skills" / name
d.mkdir(parents=True)
(d / "SKILL.md").write_text(f"# {name}")

# Mock the dependency
dep = DependencyReference.parse("testorg/agent-library/modernisation")
apm_package = Mock()
apm_package.get_apm_dependencies.return_value = [dep]

result = self.integrator.sync_integration(apm_package, self.project_root)

# Neither should be removed
assert result['files_removed'] == 0
assert (self.project_root / ".github" / "skills" / "modernisation").exists()
assert (self.project_root / ".github" / "skills" / "azure-naming").exists()
Loading