Fix duplicated skills/ directory nesting in plugin pack

[Ai-chan-0411]

Description

When running apm pack --format plugin with a dependency whose virtual_path starts with skills/ (e.g. github/awesome-copilot/skills/javascript-typescript-jest), the bare skill collector (_collect_bare_skill) used the full virtual_path as the slug. Since the function already prepends skills/ to the output path, this produced skills/skills/javascript-typescript-jest/SKILL.md instead of the expected skills/javascript-typescript-jest/SKILL.md.

I ran into this while testing a plugin that depends on github/awesome-copilot/skills/javascript-typescript-jest — the packed output had the skill files nested one level too deep, which meant the plugin host couldn't discover them.

The fix strips the leading skills/ prefix from virtual_path before using it as the directory slug, preventing the double nesting. Added a regression test that verifies virtual paths with a skills/ prefix produce the correct flat structure.

Fixes #719

Type of change

• Bug fix
• New feature
• Documentation
• Maintenance / refactor

Testing

• Tested locally
• All existing tests pass (65/65 in test_plugin_exporter.py)
• Added tests for new functionality (if applicable)

[Ai-chan-0411]

@microsoft-github-policy-service agree

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes incorrect skills/skills/... directory nesting when packing plugins that depend on virtual skills whose virtual_path already includes a skills/ prefix.

Changes:

• Strip a leading skills/ prefix from virtual_path before using it as the output slug in _collect_bare_skill
• Add a regression unit test to ensure virtual_path="skills/..." produces a flat skills/... output structure

Show a summary per file

File	Description
src/apm_cli/bundle/plugin_exporter.py	Normalizes virtual_path to avoid double skills/ nesting when building output paths
tests/unit/test_plugin_exporter.py	Adds regression coverage for virtual_path values starting with skills/

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 2

[Copilot]

slug = slug[len("skills/"):] can leave an unintended leading slash if virtual_path contains multiple slashes (e.g. "skills//foo" becomes "/foo"), producing paths like "skills//foo/...". Consider normalizing by removing the prefix and then stripping leading slashes (or splitting into path segments and dropping an initial "skills" segment) so slug never starts with /.

Suggested change

	slug = slug[len("skills/"):]
	slug = slug[len("skills/"):].lstrip("/")

[Copilot]

The test assertion is a bit loose (it only checks the directory prefix). Since this test creates only SKILL.md, asserting the exact expected relative path (e.g. equality against "skills/javascript-typescript-jest/SKILL.md") would make the regression test stricter and easier to interpret. Also, out: list = [] is very generic; if this file uses type hints elsewhere, annotating out with the expected element type would improve readability.

Suggested change

	out: list = []
	_collect_bare_skill(tmp_path, dep, out)
	rel_paths = [r for _, r in out]
	# Should be skills/javascript-typescript-jest/SKILL.md, NOT skills/skills/...
	assert any(r.startswith("skills/javascript-typescript-jest/") for r in rel_paths)
	out: list[tuple[Path, str]] = []
	_collect_bare_skill(tmp_path, dep, out)
	rel_paths = [r for _, r in out]
	# Should be skills/javascript-typescript-jest/SKILL.md, NOT skills/skills/...
	assert rel_paths == ["skills/javascript-typescript-jest/SKILL.md"]

[Ai-chan-0411]

Thanks for the Copilot review — both points addressed in f75ad61:

1. Leading slash after skills/ strip (plugin_exporter.py):
Added .lstrip("/") after the slice so "skills//foo" → "foo" instead of "/foo". The slug computation now reads:

slug = slug[len("skills/"):].lstrip("/")

2. Loose assertion in test_virtual_path_used_as_slug (test_plugin_exporter.py):
Changed from prefix check to exact equality:

assert rel_paths == ["skills/frontend-design/SKILL.md"]

Applied the same tightening to test_virtual_path_with_skills_prefix_no_double_nesting, and added a new regression test test_virtual_path_with_skills_double_slash_no_leading_slash that exercises the double-slash edge case directly.

All 66 tests pass locally.