Skip to content

Commit 69ffcd4

Browse files
committed
fix: Claude integration improvements
- Fix virtual subdirectory packages generating skills (mcp-builder, etc.) - Add native Claude Skills support - copy entire SKILL.md folder for packages with existing skills - Add APM metadata to SKILL.md for orphan detection during sync - Clean up empty parent directories after uninstall - Simplify skill folder naming to use source folder name directly - Fix CLAUDE.md to only contain instructions (not agents) - Agents go to .github/agents/ as separate files, matching AGENTS.md behavior - Remove Workflows section from CLAUDE.md generation - Update tests to verify agents are excluded Files changed: - src/apm_cli/cli.py: Empty parent dir cleanup after uninstall - src/apm_cli/integration/skill_integrator.py: Native skills, metadata, naming - src/apm_cli/compilation/claude_formatter.py: Remove agents from CLAUDE.md - tests/: Updated to match new behavior
1 parent 72354f5 commit 69ffcd4

5 files changed

Lines changed: 178 additions & 82 deletions

File tree

‎src/apm_cli/cli.py‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -941,6 +941,15 @@ def uninstall(ctx, packages, dry_run):
941941
shutil.rmtree(package_path)
942942
_rich_info(f"✓ Removed {package} from apm_modules/")
943943
removed_from_modules += 1
944+
945+
# Cleanup empty parent directories up to apm_modules/
946+
parent = package_path.parent
947+
while parent != apm_modules_dir and parent.exists():
948+
if not any(parent.iterdir()):
949+
parent.rmdir()
950+
parent = parent.parent
951+
else:
952+
break
944953
except Exception as e:
945954
_rich_error(
946955
f"✗ Failed to remove {package} from apm_modules/: {e}"

‎src/apm_cli/compilation/claude_formatter.py‎

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,9 @@ class ClaudeFormatter:
5959
Generates CLAUDE.md files following Claude's Memory format with:
6060
- @import syntax for dependencies
6161
- Grouped project standards from instructions
62-
- Workflows section for agents/roles
62+
63+
Note: Agents/workflows are handled separately as .github/agents/ files,
64+
not included in CLAUDE.md (same as AGENTS.md behavior).
6365
"""
6466

6567
def __init__(self, base_dir: str = "."):
@@ -306,30 +308,9 @@ def _generate_claude_content(
306308
sections.append(content)
307309
sections.append("")
308310

309-
# Workflows section (agents/roles - only for root)
310-
if placement.agents:
311-
sections.append("# Workflows")
312-
sections.append("")
313-
314-
for agent in placement.agents:
315-
# Agent name as heading
316-
sections.append(f"## {agent.name}")
317-
if agent.description:
318-
sections.append(f"*{agent.description}*")
319-
sections.append("")
320-
321-
# Agent content
322-
content = agent.content.strip()
323-
if content:
324-
try:
325-
rel_path = agent.file_path.relative_to(self.base_dir)
326-
except ValueError:
327-
rel_path = agent.file_path
328-
329-
sections.append(f"<!-- Source: {rel_path} -->")
330-
sections.append(content)
331-
sections.append(f"<!-- End source: {rel_path} -->")
332-
sections.append("")
311+
# Note: CLAUDE.md only contains instructions (Project Standards).
312+
# Agents/workflows are NOT included - they go to .github/agents/ as separate files.
313+
# This matches AGENTS.md behavior which also only contains instructions.
333314

334315
# Footer
335316
sections.append("---")
@@ -355,14 +336,12 @@ def _compile_stats(
355336
"""
356337
total_instructions = sum(len(p.instructions) for p in placements)
357338
total_patterns = sum(len(p.coverage_patterns) for p in placements)
358-
total_agents = sum(len(p.agents) for p in placements)
359339
total_deps = sum(len(p.dependencies) for p in placements)
360340

361341
return {
362342
"claude_files_generated": len(placements),
363343
"total_instructions_placed": total_instructions,
364344
"total_patterns_covered": total_patterns,
365-
"total_agents_included": total_agents,
366345
"total_dependencies": total_deps,
367346
"primitives_found": primitives.count(),
368347
}

‎src/apm_cli/integration/skill_integrator.py‎

Lines changed: 121 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,110 @@ def _generate_skill_file(self, package_info, primitives: dict, skill_dir: Path)
514514

515515
return files_copied
516516

517+
def _integrate_native_claude_skill(
518+
self, package_info, project_root: Path, source_skill_md: Path
519+
) -> SkillIntegrationResult:
520+
"""Copy a native Claude Skill (with existing SKILL.md) to .claude/skills/.
521+
522+
For packages that already have a SKILL.md at their root (like those from
523+
awesome-claude-skills), we copy the entire skill folder rather than
524+
regenerating from .apm/ primitives.
525+
526+
The skill folder name is the source folder name (e.g., `mcp-builder`).
527+
528+
We add APM tracking metadata to the copied SKILL.md so sync_integration
529+
can properly identify and clean up orphaned skills.
530+
531+
Args:
532+
package_info: PackageInfo object with package metadata
533+
project_root: Root directory of the project
534+
source_skill_md: Path to the source SKILL.md file
535+
536+
Returns:
537+
SkillIntegrationResult: Results of the integration operation
538+
"""
539+
package_path = package_info.install_path
540+
541+
# Use the source folder name as the skill name
542+
# e.g., apm_modules/ComposioHQ/awesome-claude-skills/mcp-builder → mcp-builder
543+
skill_name = package_path.name
544+
545+
skill_dir = project_root / ".claude" / "skills" / skill_name
546+
target_skill_md = skill_dir / "SKILL.md"
547+
548+
# Check if we need to update
549+
skill_created = False
550+
skill_updated = False
551+
skill_skipped = False
552+
553+
if target_skill_md.exists():
554+
# Check existing metadata for version/commit changes
555+
existing_metadata = self._parse_skill_metadata(target_skill_md)
556+
current_version = package_info.package.version
557+
current_commit = (
558+
package_info.resolved_reference.resolved_commit
559+
if package_info.resolved_reference
560+
else "unknown"
561+
)
562+
563+
if (existing_metadata.get('Version') == current_version and
564+
existing_metadata.get('Commit') == current_commit):
565+
skill_skipped = True
566+
else:
567+
skill_updated = True
568+
else:
569+
skill_created = True
570+
571+
files_copied = 0
572+
573+
if skill_created or skill_updated:
574+
# Remove existing skill directory if updating
575+
if skill_dir.exists():
576+
shutil.rmtree(skill_dir)
577+
578+
# Copy the entire package directory to the skill location
579+
shutil.copytree(package_path, skill_dir)
580+
581+
# Add APM tracking metadata to the SKILL.md for orphan detection
582+
self._add_apm_metadata_to_skill(target_skill_md, package_info)
583+
584+
# Count files copied
585+
files_copied = sum(1 for _ in skill_dir.rglob('*') if _.is_file())
586+
587+
return SkillIntegrationResult(
588+
skill_created=skill_created,
589+
skill_updated=skill_updated,
590+
skill_skipped=skill_skipped,
591+
skill_path=target_skill_md if (skill_created or skill_updated) else None,
592+
references_copied=files_copied,
593+
links_resolved=0
594+
)
595+
596+
def _add_apm_metadata_to_skill(self, skill_path: Path, package_info) -> None:
597+
"""Add APM tracking metadata to a SKILL.md file.
598+
599+
This ensures sync_integration can identify APM-managed skills for cleanup.
600+
"""
601+
post = frontmatter.load(skill_path)
602+
603+
# Add nested metadata for APM tracking
604+
if 'metadata' not in post.metadata:
605+
post.metadata['metadata'] = {}
606+
607+
post.metadata['metadata']['apm_package'] = package_info.get_canonical_dependency_string()
608+
post.metadata['metadata']['apm_version'] = package_info.package.version
609+
post.metadata['metadata']['apm_commit'] = (
610+
package_info.resolved_reference.resolved_commit
611+
if package_info.resolved_reference
612+
else "unknown"
613+
)
614+
post.metadata['metadata']['apm_installed_at'] = (
615+
package_info.installed_at or datetime.now().isoformat()
616+
)
617+
618+
with open(skill_path, 'w', encoding='utf-8') as f:
619+
f.write(frontmatter.dumps(post))
620+
517621
def integrate_package_skill(self, package_info, project_root: Path) -> SkillIntegrationResult:
518622
"""Generate SKILL.md for a package in .claude/skills/ directory.
519623
@@ -554,7 +658,12 @@ def integrate_package_skill(self, package_info, project_root: Path) -> SkillInte
554658

555659
package_path = package_info.install_path
556660

557-
# Discover all primitives
661+
# Check if this is a native Claude Skill (already has SKILL.md at root)
662+
source_skill_md = package_path / "SKILL.md"
663+
if source_skill_md.exists():
664+
return self._integrate_native_claude_skill(package_info, project_root, source_skill_md)
665+
666+
# Discover all primitives for APM packages without SKILL.md
558667
instruction_files = self.find_instruction_files(package_path)
559668
agent_files = self.find_agent_files(package_path)
560669
context_files = self.find_context_files(package_path)
@@ -582,11 +691,9 @@ def integrate_package_skill(self, package_info, project_root: Path) -> SkillInte
582691
)
583692

584693
# Determine target paths - write to .claude/skills/{skill-name}/
585-
# Use skill_name_from_dependency for unique, consistent names:
586-
# - owner/repo → owner-repo
587-
# - owner/repo/skill → owner-repo-skill
588-
canonical_dep = package_info.get_canonical_dependency_string()
589-
skill_name = skill_name_from_dependency(canonical_dep)
694+
# Use the install folder name for simplicity and consistency
695+
# e.g., apm_modules/danielmeppiel/design-guidelines → design-guidelines
696+
skill_name = package_path.name
590697
skill_dir = project_root / ".claude" / "skills" / skill_name
591698
skill_dir.mkdir(parents=True, exist_ok=True)
592699

@@ -632,6 +739,7 @@ def sync_integration(self, apm_package, project_root: Path) -> Dict[str, int]:
632739
"""Sync .claude/skills/ with currently installed packages.
633740
634741
Removes skill directories for packages that are no longer installed.
742+
Uses apm_package metadata in SKILL.md to identify APM-managed skills.
635743
636744
Args:
637745
apm_package: APMPackage with current dependencies
@@ -644,27 +752,24 @@ def sync_integration(self, apm_package, project_root: Path) -> Dict[str, int]:
644752
if not skills_dir.exists():
645753
return {'files_removed': 0, 'errors': 0}
646754

647-
# Get list of currently installed package skill names
648-
# Use skill_name_from_dependency to match how integrate_package_skill names them
649-
installed_skill_names = set()
755+
# Get canonical dependency strings for all installed packages
756+
installed_packages = set()
650757
for dep in apm_package.get_apm_dependencies():
651-
skill_name = skill_name_from_dependency(dep.get_canonical_dependency_string())
652-
installed_skill_names.add(skill_name)
758+
installed_packages.add(dep.get_canonical_dependency_string())
653759

654-
# Find orphaned skill directories (those with -apm suffix pattern or all if needed)
760+
# Find orphaned skill directories by checking apm_package metadata
655761
files_removed = 0
656762
errors = 0
657763

658764
for skill_subdir in skills_dir.iterdir():
659765
if skill_subdir.is_dir():
660-
# Check if this skill came from APM (has apm_package in metadata)
661766
skill_md = skill_subdir / "SKILL.md"
662767
if skill_md.exists():
663768
try:
664769
metadata = self._parse_skill_metadata(skill_md)
665-
if metadata.get('Package'): # Has APM metadata
666-
skill_name = skill_subdir.name
667-
if skill_name not in installed_skill_names:
770+
apm_package_ref = metadata.get('Package')
771+
if apm_package_ref: # This is an APM-managed skill
772+
if apm_package_ref not in installed_packages:
668773
# Remove orphaned skill directory
669774
shutil.rmtree(skill_subdir)
670775
files_removed += 1

‎tests/unit/compilation/test_claude_formatter.py‎

Lines changed: 40 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -257,8 +257,12 @@ def test_no_dependencies_section_when_no_apm_modules(self, tmp_path):
257257
assert deps == []
258258

259259

260-
class TestAgentsAndWorkflows:
261-
"""Tests for agents/workflows inclusion in CLAUDE.md."""
260+
class TestAgentsExcludedFromClaudeMd:
261+
"""Tests verifying agents/workflows are NOT included in CLAUDE.md.
262+
263+
Agents are handled separately via .github/agents/ files and .claude/skills/
264+
folders - they should NOT be embedded in CLAUDE.md (same as AGENTS.md).
265+
"""
262266

263267
@pytest.fixture
264268
def temp_project(self):
@@ -267,8 +271,8 @@ def temp_project(self):
267271
yield Path(temp_dir).resolve()
268272
shutil.rmtree(temp_dir, ignore_errors=True)
269273

270-
def test_agents_included_in_workflows_section(self, temp_project):
271-
"""Test that agents/chatmodes are included in Workflows section."""
274+
def test_agents_not_in_claude_md(self, temp_project):
275+
"""Test that agents/chatmodes are NOT included in CLAUDE.md."""
272276
formatter = ClaudeFormatter(str(temp_project))
273277
primitives = PrimitiveCollection()
274278

@@ -298,13 +302,14 @@ def test_agents_included_in_workflows_section(self, temp_project):
298302

299303
content = result.content_map[formatter.base_dir / "CLAUDE.md"]
300304

301-
assert "# Workflows" in content
302-
assert "## code-reviewer" in content
303-
assert "*Expert code reviewer*" in content
304-
assert "You are an expert code reviewer." in content
305+
# Workflows section should NOT exist - agents go to .github/agents/
306+
assert "# Workflows" not in content
307+
assert "## code-reviewer" not in content
308+
# Instructions should still be there
309+
assert "Test content" in content
305310

306-
def test_agents_only_in_root_claude_md(self, temp_project):
307-
"""Test that agents are only included in root CLAUDE.md."""
311+
def test_claude_md_only_contains_instructions(self, temp_project):
312+
"""Test that CLAUDE.md only contains instructions, not agents."""
308313
formatter = ClaudeFormatter(str(temp_project))
309314
primitives = PrimitiveCollection()
310315

@@ -319,35 +324,30 @@ def test_agents_only_in_root_claude_md(self, temp_project):
319324
primitives.add_primitive(chatmode)
320325

321326
instruction = Instruction(
322-
name="test",
323-
file_path=temp_project / "test.instructions.md",
324-
description="Test",
325-
apply_to="src/**/*.py",
326-
content="Test content",
327+
name="python-standards",
328+
file_path=temp_project / "python.instructions.md",
329+
description="Python standards",
330+
apply_to="**/*.py",
331+
content="Use type hints for all functions.",
327332
author="test"
328333
)
329334
primitives.add_primitive(instruction)
330335

331-
# Create subdirectory placement using resolved paths
332-
src_dir = formatter.base_dir / "src"
333-
src_dir.mkdir(exist_ok=True)
334-
335-
placement_map = {
336-
formatter.base_dir: [instruction],
337-
src_dir: [instruction]
338-
}
336+
placement_map = {formatter.base_dir: [instruction]}
339337
result = formatter.format_distributed(primitives, placement_map)
340338

341-
# Root should have workflows
342-
root_content = result.content_map[formatter.base_dir / "CLAUDE.md"]
343-
assert "# Workflows" in root_content
339+
content = result.content_map[formatter.base_dir / "CLAUDE.md"]
340+
341+
# Should have instructions
342+
assert "Use type hints for all functions." in content
343+
assert "**/*.py" in content
344344

345-
# Subdirectory should not have workflows
346-
src_content = result.content_map[src_dir / "CLAUDE.md"]
347-
assert "# Workflows" not in src_content
345+
# Should NOT have agent content
346+
assert "You are a reviewer." not in content
347+
assert "Workflows" not in content
348348

349-
def test_multiple_agents_included(self, temp_project):
350-
"""Test that multiple agents are all included."""
349+
def test_multiple_chatmodes_excluded(self, temp_project):
350+
"""Test that multiple chatmodes are all excluded from CLAUDE.md."""
351351
formatter = ClaudeFormatter(str(temp_project))
352352
primitives = PrimitiveCollection()
353353

@@ -375,7 +375,7 @@ def test_multiple_agents_included(self, temp_project):
375375
file_path=temp_project / "test.instructions.md",
376376
description="Test",
377377
apply_to="**/*.py",
378-
content="Test",
378+
content="Test instruction content",
379379
author="test"
380380
)
381381
primitives.add_primitive(instruction)
@@ -385,10 +385,14 @@ def test_multiple_agents_included(self, temp_project):
385385

386386
content = result.content_map[formatter.base_dir / "CLAUDE.md"]
387387

388-
assert "## reviewer" in content
389-
assert "## architect" in content
390-
assert "Review code." in content
391-
assert "Design systems." in content
388+
# Agents should NOT be in CLAUDE.md
389+
assert "## reviewer" not in content
390+
assert "## architect" not in content
391+
assert "Review code." not in content
392+
assert "Design systems." not in content
393+
394+
# But instructions should be there
395+
assert "Test instruction content" in content
392396

393397

394398
class TestGenerateCommands:

‎tests/unit/integration/test_skill_integrator.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,9 @@ def teardown_method(self):
134134
def _get_skill_path(self, package_info) -> Path:
135135
"""Get the expected skill directory path for a package.
136136
137-
Uses skill_name_from_dependency to match the actual integrate_package_skill behavior.
137+
Uses the install folder name for simplicity and consistency.
138138
"""
139-
canonical = package_info.get_canonical_dependency_string()
140-
skill_name = skill_name_from_dependency(canonical)
139+
skill_name = package_info.install_path.name
141140
return self.project_root / ".claude" / "skills" / skill_name
142141

143142
# ========== should_integrate tests ==========

0 commit comments

Comments
 (0)