From 6bb86fe4e2c83fe7809a2d6a673d34489a347ffd Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 9 Jun 2026 15:50:22 +0200 Subject: [PATCH 1/2] fix(install): honor skill subset for plugin skills Thread the install skill subset through standalone sub-skill promotion so plugin-manifest skill collections install only the requested skills. Normalize subset path values to their leaf skill names for promotion filtering. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../content/docs/getting-started/migration.md | 11 ++--- .../src/content/docs/reference/cli/install.md | 2 +- .../content/docs/reference/package-types.md | 4 +- .../.apm/skills/apm-usage/commands.md | 4 +- src/apm_cli/integration/skill_integrator.py | 40 +++++++++++++++---- .../test_skill_integrator_phase3w4.py | 40 +++++++++++++++++++ 6 files changed, 85 insertions(+), 16 deletions(-) diff --git a/docs/src/content/docs/getting-started/migration.md b/docs/src/content/docs/getting-started/migration.md index 09ce9757f..225d1a98f 100644 --- a/docs/src/content/docs/getting-started/migration.md +++ b/docs/src/content/docs/getting-started/migration.md @@ -71,7 +71,8 @@ apm install ``` The `--skill` flag is repeatable. Your selection is written to `apm.yml` and -`apm.lock.yaml` so the exact subset is reproducible on every machine. +`apm.lock.yaml` so the exact subset is reproducible on every machine. For +plugin manifest collections, pass either the skill name or the manifest path. ```bash # Pick two skills, then reset to all @@ -105,7 +106,7 @@ skill collection layout reference. :::caution[Behavior change] Skills for **Copilot, Cursor, OpenCode, Codex, and Gemini** now deploy to `.agents/skills/` by default instead of per-client directories (`.github/skills/`, `.cursor/skills/`, `.gemini/skills/`, etc.). This matches the `.agents/` discovery path documented by all five clients and eliminates redundant copies when targeting multiple clients. -**Claude is unchanged** — its skills continue to deploy to `.claude/skills/`. +**Claude is unchanged** - its skills continue to deploy to `.claude/skills/`. To restore the previous per-client layout, pass `--legacy-skill-paths` to any command, or set the `APM_LEGACY_SKILL_PATHS=1` environment variable. ::: @@ -117,12 +118,12 @@ When you upgrade APM and run `apm install`, the tool automatically detects legac **What happens:** - Old per-client skill files are deleted after the new `.agents/skills/` files are written - The lockfile is updated to reflect the new paths -- The migration is idempotent — running `apm install` again is a no-op +- The migration is idempotent - running `apm install` again is a no-op - Foreign / hand-authored skills outside the lockfile are never touched **What does NOT migrate:** -- `.claude/skills/` — Claude is not part of the convergence -- `.codex/skills/` — Codex was already on `.agents/skills/` before this change +- `.claude/skills/` - Claude is not part of the convergence +- `.codex/skills/` - Codex was already on `.agents/skills/` before this change - Any file not tracked in `apm.lock.yaml` **If a collision is detected** (e.g., a foreign file already exists at the destination `.agents/skills/` path with different content), the migration aborts entirely with a clear error. Use `--legacy-skill-paths` to skip migration and keep per-client paths. diff --git a/docs/src/content/docs/reference/cli/install.md b/docs/src/content/docs/reference/cli/install.md index 769a355a0..4cbaf385c 100644 --- a/docs/src/content/docs/reference/cli/install.md +++ b/docs/src/content/docs/reference/cli/install.md @@ -80,7 +80,7 @@ Transport env vars: `APM_GIT_PROTOCOL` (`ssh` or `https`) sets the default initi | Flag | Default | Description | |---|---|---| -| `--skill NAME` | all | Install only named skill(s) from a `SKILL_BUNDLE` package. Repeatable. The selection is persisted to `apm.yml` and `apm.lock.yaml`. Use `--skill '*'` to reset and install all. | +| `--skill NAME` | all | Install only named skill(s) from a skill collection (`SKILL_BUNDLE` or plugin manifest). Repeatable. For plugin manifests, `NAME` may be the skill name or manifest path, such as `skills/productivity/grill-me`. The selection is persisted to `apm.yml` and `apm.lock.yaml`. Use `--skill '*'` to reset and install all. | | `--as ALIAS` | bundle id | Override the log/display label for a local-bundle install. Only valid with a single local-bundle `PACKAGE_REF`. | ### MCP server entry (use only with `--mcp`) diff --git a/docs/src/content/docs/reference/package-types.md b/docs/src/content/docs/reference/package-types.md index 78893f146..000b90470 100644 --- a/docs/src/content/docs/reference/package-types.md +++ b/docs/src/content/docs/reference/package-types.md @@ -200,7 +200,9 @@ my-plugin/ ``` **What gets installed:** each artifact listed in `plugin.json` is mapped to -the appropriate runtime directory via `_map_plugin_artifacts`. +the appropriate runtime directory via `_map_plugin_artifacts`. Use `--skill` +to cherry-pick plugin skills by leaf name or manifest path, such as +`skills/productivity/grill-me`. **When to choose:** you already have a Claude plugin and want APM to consume it without restructuring. diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index b6ea5bc39..c4d36f36a 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -10,7 +10,7 @@ | Command | Purpose | Key flags | |---------|---------|-----------| -| `apm install [PKGS...]` | Install APM and MCP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)) | `--update` (deprecated; prefer `apm update`) refresh refs, `--refresh` re-fetch all deps from upstream and re-resolve all ref pins, `--force` overwrite (does NOT refresh refs; use `apm update` for that), `--frozen` CI-safe install that fails fast when `apm.lock.yaml` is missing or out of sync with `apm.yml` (mutually exclusive with `--update`; structural presence check only -- use `apm audit` for SHA integrity), `--dry-run`, `--verbose`, `--only [apm\|mcp]`, `--target` (comma-separated, e.g. `--target claude,cursor`; highest-priority entry in the resolution chain `--target` > apm.yml `targets:` > auto-detect; `--target all` deprecated, see `apm compile --all`; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`), `--dev`, `-g` global (MCP deploys only to user-scope runtimes: Copilot CLI, Codex CLI, JetBrains Copilot), `--trust-transitive-mcp`, `--parallel-downloads N`, `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skill(s) from SKILL_BUNDLE (repeatable; persisted in apm.yml; `'*'` resets to all), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry (NAME goes through the same `--target` > `targets:` > auto-detect resolver as APM packages, so a project whitelisting `targets: [copilot]` will not write `.cursor/mcp.json` even if `.cursor/` exists; `apm install -g --mcp NAME` writes user-scope and bypasses the project-scope gate by design), `--transport`, `--url`, `--env KEY=VAL`, `--header KEY=VAL`, `--mcp-version`, `--registry URL` custom MCP registry, `--root DIR` redirect writes (`apm_modules/`, lockfile, `.gitignore`, integrated harness files) under DIR while `apm.yml`/`.apm/`/local deps resolve from `$PWD` (mirrors `pip install --target`; created if missing; not valid with `-g`/`--global`, which exits 2) | +| `apm install [PKGS...]` | Install APM and MCP dependencies (supports APM packages, Claude skills (SKILL.md), and plugin collections (plugin.json)) | `--update` (deprecated; prefer `apm update`) refresh refs, `--refresh` re-fetch all deps from upstream and re-resolve all ref pins, `--force` overwrite (does NOT refresh refs; use `apm update` for that), `--frozen` CI-safe install that fails fast when `apm.lock.yaml` is missing or out of sync with `apm.yml` (mutually exclusive with `--update`; structural presence check only -- use `apm audit` for SHA integrity), `--dry-run`, `--verbose`, `--only [apm\|mcp]`, `--target` (comma-separated, e.g. `--target claude,cursor`; highest-priority entry in the resolution chain `--target` > apm.yml `targets:` > auto-detect; `--target all` deprecated, see `apm compile --all`; use `copilot-cowork` with `--global` after `apm experimental enable copilot-cowork`), `--dev`, `-g` global (MCP deploys only to user-scope runtimes: Copilot CLI, Codex CLI, JetBrains Copilot), `--trust-transitive-mcp`, `--parallel-downloads N`, `--allow-insecure`, `--allow-insecure-host HOSTNAME`, `--skill NAME` install named skill(s) from a skill collection (SKILL_BUNDLE or plugin manifest; repeatable; plugin manifests accept a leaf name or manifest path; persisted in apm.yml; `'*'` resets to all), `--legacy-skill-paths` restore per-client skill dirs, `--mcp NAME` add MCP entry (NAME goes through the same `--target` > `targets:` > auto-detect resolver as APM packages, so a project whitelisting `targets: [copilot]` will not write `.cursor/mcp.json` even if `.cursor/` exists; `apm install -g --mcp NAME` writes user-scope and bypasses the project-scope gate by design), `--transport`, `--url`, `--env KEY=VAL`, `--header KEY=VAL`, `--mcp-version`, `--registry URL` custom MCP registry, `--root DIR` redirect writes (`apm_modules/`, lockfile, `.gitignore`, integrated harness files) under DIR while `apm.yml`/`.apm/`/local deps resolve from `$PWD` (mirrors `pip install --target`; created if missing; not valid with `-g`/`--global`, which exits 2) | | `apm targets` | Show resolved deployment targets for the current project (Click group; reads filesystem signals; works with or without `apm.yml`) | `--all` also include the `agent-skills` meta-target (only meaningful with `--json`), `--json` machine-readable output. No provenance line is printed (the table is the provenance). | | `apm uninstall PKGS...` | Remove packages (accepts `owner/repo` or `name@marketplace`) | `--dry-run`, `-g` global | | `apm prune` | Remove orphaned packages | `--dry-run` | @@ -91,7 +91,7 @@ Behind `apm experimental enable registries`. Pushes a package version to a REST- | Command | Purpose | Key flags | |---------|---------|-----------| -| `apm publish` | Auto-pack a flat registry zip archive (`apm.yml` + `.apm/` + `README.md`/`CHANGELOG.md`/`LICENSE` when present) and upload to a configured registry via `PUT /v1/packages/{owner}/{repo}/versions/{version}`. Different layout from `apm pack` (no `plugin.json` wrapper). | `--registry NAME` (required when multiple registries are configured), `--package OWNER/REPO` (**required** — registry package identity, e.g. `acme/my-skill`), `--zip PATH` (skip auto-pack and upload a pre-built `.zip`), `--dry-run`, `-v`/`--verbose` | +| `apm publish` | Auto-pack a flat registry zip archive (`apm.yml` + `.apm/` + `README.md`/`CHANGELOG.md`/`LICENSE` when present) and upload to a configured registry via `PUT /v1/packages/{owner}/{repo}/versions/{version}`. Different layout from `apm pack` (no `plugin.json` wrapper). | `--registry NAME` (required when multiple registries are configured), `--package OWNER/REPO` (**required** - registry package identity, e.g. `acme/my-skill`), `--zip PATH` (skip auto-pack and upload a pre-built `.zip`), `--dry-run`, `-v`/`--verbose` | Examples: diff --git a/src/apm_cli/integration/skill_integrator.py b/src/apm_cli/integration/skill_integrator.py index 3264bdd37..8e380ffb9 100644 --- a/src/apm_cli/integration/skill_integrator.py +++ b/src/apm_cli/integration/skill_integrator.py @@ -555,6 +555,25 @@ def _resolve_markdown_links_in_skill_bundle( links_resolved += count return links_resolved + @staticmethod + def _skill_subset_name_filter(skill_subset) -> set[str] | None: + """Return promotion filter tokens for --skill subset values.""" + if not skill_subset: + return None + + name_filter: set[str] = set() + for skill_name in skill_subset: + raw_name = str(skill_name).strip() + if not raw_name: + continue + normalized_path = raw_name.replace("\\", "/") + leaf_name = Path(normalized_path).name + name_filter.add(raw_name) + name_filter.add(normalized_path) + if leaf_name: + name_filter.add(leaf_name) + return name_filter or None + @staticmethod def _promote_sub_skills( sub_skills_dir: Path, @@ -568,7 +587,7 @@ def _promote_sub_skills( force: bool = False, project_root: Path | None = None, logger=None, - name_filter: "set | None" = None, + name_filter: set[str] | None = None, link_rewriter: "SkillIntegrator | None" = None, ) -> tuple[int, list[Path]]: """Promote sub-skills from .apm/skills/ to top-level skill entries. @@ -616,7 +635,7 @@ def _promote_sub_skills( target = target_skills_root / sub_name rel_path = f"{rel_prefix}/{sub_name}" if target.exists(): - # Content-identical → skip entirely (no copy, no warning) + # Content-identical: skip entirely (no copy, no warning) if SkillIntegrator.is_skill_dir_identical_to_source(sub_skill_path, target): promoted += 1 deployed.append(target) @@ -630,7 +649,7 @@ def _promote_sub_skills( is_self_overwrite = prev_owner is not None and prev_owner == parent_name if managed_files is not None and not is_managed and not is_self_overwrite: - # User-authored skill — respect force flag + # User-authored skill: respect force flag if not force: if diagnostics is not None: diagnostics.skip(rel_path, package=parent_name) @@ -649,7 +668,7 @@ def _promote_sub_skills( ) except ImportError: pass - continue # SKIP — protect user content + continue # SKIP: protect user content if warn and not is_self_overwrite: if diagnostics is not None: @@ -741,6 +760,7 @@ def _promote_sub_skills_standalone( force: bool = False, logger=None, targets=None, + skill_subset=None, ) -> tuple[int, list[Path]]: """Promote sub-skills from a package that is NOT itself a skill. @@ -753,6 +773,7 @@ def _promote_sub_skills_standalone( package_info: PackageInfo object with package metadata. project_root: Root directory of the project. targets: Optional explicit list of TargetProfile objects. + skill_subset: Optional tuple of skill names or paths to install (None = all). Returns: tuple[int, list[Path]]: (count of promoted sub-skills, list of deployed dirs) @@ -770,6 +791,7 @@ def _promote_sub_skills_standalone( parent_name = package_path.name owned_by = self._build_skill_ownership_map(project_root) + name_filter = self._skill_subset_name_filter(skill_subset) count = 0 all_deployed: list[Path] = [] seen_skill_dirs: set[Path] = set() @@ -810,6 +832,7 @@ def _promote_sub_skills_standalone( managed_files=managed_files if is_primary else None, force=force, project_root=project_root, + name_filter=name_filter, link_rewriter=self, ) if is_primary: @@ -1104,8 +1127,8 @@ def _integrate_skill_bundle( any_created = False seen_skill_dirs: set[Path] = set() - # Convert skill_subset tuple to a set for O(1) lookup - _name_filter = set(skill_subset) if skill_subset else None + # Convert skill_subset tuple to promotion filter tokens for O(1) lookup. + _name_filter = self._skill_subset_name_filter(skill_subset) for idx, target in enumerate(targets): if not target.supports("skills"): @@ -1189,6 +1212,7 @@ def integrate_package_skill( package_info: PackageInfo object with package metadata project_root: Root directory of the project targets: Optional explicit list of TargetProfile objects. + skill_subset: Optional tuple of skill names or paths to install (None = all). Returns: SkillIntegrationResult: Results of the integration operation @@ -1207,6 +1231,7 @@ def integrate_package_skill( force=force, logger=logger, targets=targets, + skill_subset=skill_subset, ) return SkillIntegrationResult( skill_created=False, @@ -1311,6 +1336,7 @@ def integrate_package_skill( force=force, logger=logger, targets=targets, + skill_subset=skill_subset, ) return self._merge_bin_paths( SkillIntegrationResult( @@ -1637,7 +1663,7 @@ def sync_integration( if ".." in rel_path: continue - # ── Cowork:// paths ────────────────────────────────── + # Cowork:// paths from apm_cli.integration.copilot_cowork_paths import COWORK_URI_SCHEME if rel_path.startswith(COWORK_URI_SCHEME): diff --git a/tests/unit/integration/test_skill_integrator_phase3w4.py b/tests/unit/integration/test_skill_integrator_phase3w4.py index 4465f27a3..f7030d94f 100644 --- a/tests/unit/integration/test_skill_integrator_phase3w4.py +++ b/tests/unit/integration/test_skill_integrator_phase3w4.py @@ -414,6 +414,46 @@ def test_overwrite_warning_with_logger(self, tmp_path: Path) -> None: class TestPromoteSubSkillsStandaloneDedup: """Second target with identical resolved path is logged+skipped.""" + def test_plugin_manifest_subset_promotes_only_selected_nested_skill( + self, tmp_path: Path + ) -> None: + """--skill path filters plugin-manifest sub-skills during promotion.""" + from apm_cli.deps.plugin_parser import _map_plugin_artifacts + from apm_cli.models.apm_package import PackageType + + plugin_root = tmp_path / "skills-repo" + selected_skill = plugin_root / "skills" / "productivity" / "grill-me" + other_skill = plugin_root / "skills" / "writing" / "summarize-me" + selected_skill.mkdir(parents=True) + other_skill.mkdir(parents=True) + (selected_skill / "SKILL.md").write_text("# Grill Me", encoding="utf-8") + (other_skill / "SKILL.md").write_text("# Summarize Me", encoding="utf-8") + + manifest = { + "name": "skills-repo", + "skills": [ + "skills/productivity/grill-me", + "skills/writing/summarize-me", + ], + } + _map_plugin_artifacts(plugin_root, plugin_root / ".apm", manifest) + + pkg_info = _make_package_info(plugin_root, package_type=PackageType.MARKETPLACE_PLUGIN) + target = _make_target(name="claude", root_dir=".claude", auto_create=True) + project_root = tmp_path / "project" + project_root.mkdir() + + result = SkillIntegrator().integrate_package_skill( + pkg_info, + project_root, + targets=[target], + skill_subset=("skills/productivity/grill-me",), + ) + + assert result.sub_skills_promoted == 1 + assert (project_root / ".claude" / "skills" / "grill-me" / "SKILL.md").exists() + assert not (project_root / ".claude" / "skills" / "summarize-me" / "SKILL.md").exists() + def test_duplicate_target_skips_with_logger(self, tmp_path: Path) -> None: pkg_dir = tmp_path / "pkg" sub_skills = pkg_dir / ".apm" / "skills" From 8f70f59653bf291fd913f8901a9412b3ad4e4686 Mon Sep 17 00:00:00 2001 From: danielmeppiel Date: Tue, 9 Jun 2026 16:03:18 +0200 Subject: [PATCH 2/2] chore(install): type skill subset filter Add the missing type annotation for the helper introduced by the subset filtering fix. This folds the python-architect panel follow-up without changing runtime behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/integration/skill_integrator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/apm_cli/integration/skill_integrator.py b/src/apm_cli/integration/skill_integrator.py index 8e380ffb9..ed1c7c6b1 100644 --- a/src/apm_cli/integration/skill_integrator.py +++ b/src/apm_cli/integration/skill_integrator.py @@ -556,7 +556,7 @@ def _resolve_markdown_links_in_skill_bundle( return links_resolved @staticmethod - def _skill_subset_name_filter(skill_subset) -> set[str] | None: + def _skill_subset_name_filter(skill_subset: tuple[str, ...] | None) -> set[str] | None: """Return promotion filter tokens for --skill subset values.""" if not skill_subset: return None