fix(install): honor --skill subset on non-package skill-collection install (closes #1707)#1709
Merged
Merged
Conversation
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>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes apm install --skill ... subset installs for plugin-manifest skill collections that are not APM packages, ensuring the selected subset is honored during standalone sub-skill promotion (closing #1707).
Changes:
- Thread
skill_subsetinto the standalone sub-skill promotion path and normalize subset entries to support both leaf names and manifest paths. - Add a regression unit test covering plugin-manifest nested-skill selection.
- Update CLI/reference docs to document subset selection for plugin manifests.
Show a summary per file
| File | Description |
|---|---|
src/apm_cli/integration/skill_integrator.py |
Adds subset normalization and passes the resulting filter into standalone sub-skill promotion so plugin-manifest installs honor --skill. |
tests/unit/integration/test_skill_integrator_phase3w4.py |
Adds a regression test validating only the selected plugin-manifest nested skill is promoted. |
packages/apm-guide/.apm/skills/apm-usage/commands.md |
Updates the apm install flag reference to describe plugin-manifest subset selection via --skill. |
docs/src/content/docs/reference/package-types.md |
Documents plugin collections’ ability to cherry-pick skills via --skill (leaf or manifest path). |
docs/src/content/docs/reference/cli/install.md |
Updates --skill flag docs to include plugin-manifest collections and manifest-path examples. |
docs/src/content/docs/getting-started/migration.md |
Notes that plugin-manifest collections accept either skill name or manifest path for --skill. |
Copilot's findings
- Files reviewed: 6/6 changed files
- Comments generated: 1
| return links_resolved | ||
|
|
||
| @staticmethod | ||
| def _skill_subset_name_filter(skill_subset) -> set[str] | None: |
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>
Collaborator
Author
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 0 | 0 | Type annotation follow-up folded; no architecture issues remain. |
| CLI Logging Expert | 0 | 0 | 0 | No CLI output or logging changes. |
| DevX UX Expert | 0 | 0 | 0 | --skill plugin manifest subset behavior and docs are coherent. |
| Supply Chain Security Expert | 0 | 0 | 0 | Filter tokens are only compared against discovered names, not used as paths. |
| OSS Growth Hacker | 0 | 0 | 0 | Selective install story is stronger for large plugin repos. |
| Doc Writer | 0 | 0 | 0 | Docs accurately describe SKILL_BUNDLE and plugin manifest subset selection. |
| Test Coverage Expert | 0 | 0 | 0 | Regression test covers the bug path; mutation-break gate passed. |
B = blocking-severity findings, R = recommended, N = nits. Counts are signal strength, not gates. The maintainer ships.
Folded in this run
- (panel) Add type annotation to
_skill_subset_name_filterparameter -- resolved in 8f70f59.
Regression-trap evidence (mutation-break gate)
tests/unit/integration/test_skill_integrator_phase3w4.py::TestPromoteSubSkillsStandaloneDedup::test_plugin_manifest_subset_promotes_only_selected_nested_skill-- deletedname_filter=name_filterguard in standalone promotion; test FAILED as expected; guard restored.
Lint contract
uv run --extra dev ruff check src/ tests/ and uv run --extra dev ruff format --check src/ tests/ both silent.
CI
gh pr checks 1709 --repo microsoft/apm --watch reports 14 successful, 1 skipped, 0 pending, 0 failing (after 0 CI fix iteration(s)).
Mergeability status
Captured from gh pr view 1709 --json mergeable,mergeStateStatus,statusCheckRollup immediately after the last push of this run.
| PR | head SHA | CEO stance | iters | folds | defers | Copilot rounds | CI | mergeable | mergeStateStatus | notes |
|---|---|---|---|---|---|---|---|---|---|---|
| #1709 | 8f70f59 |
ship_now | 1 | 1 | 0 | 2 | green | MERGEABLE | BLOCKED | pending required review |
Convergence
1 outer iteration(s); 2 Copilot round(s). Final panel verdict: ship_now.
Ready for maintainer review.
Full per-persona findings
No remaining findings after shepherd fold.
This panel is advisory. It does not block merge. Re-apply the panel-review label after addressing feedback to re-run.
sergio-sisternes-epam
pushed a commit
that referenced
this pull request
Jun 9, 2026
Resolve conflict in skill_integrator.py from #1709 (honor --skill subset on non-package skill-collection install). My refactor extracted skill promotion into skill_deploy.py; main's #1709 edited the inline original. Resolution: keep HEAD's thin-delegator structure; port #1709 semantics into the relocated impl: - Add _skill_subset_name_filter() normalization helper to skill_deploy.py (matches --skill by raw name, normalized path, or leaf name). - Thread skill_subset -> name_filter through _promote_sub_skills_standalone (the #1707 fix: standalone/non-package-skill path now honors --skill). - skill_orchestrate.py: pass skill_subset on both standalone promotion routes (MARKETPLACE_PLUGIN / no-root-SKILL.md packages). - _integrate_skill_bundle uses the normalized helper. Shadow gate green: ruff, ruff format, pylint R0801 10.00/10, auth-signals, import smoke, 378 targeted tests pass. Files within 800-line guard (skill_integrator.py 458, skill_deploy.py 789). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sergio-sisternes-epam
pushed a commit
that referenced
this pull request
Jun 9, 2026
The #1709 port left two copies of the subset-normalization helper: a module function in skill_deploy.py and an identical SkillIntegrator staticmethod. CI's pylint R0801 guard (evaluated on the merge commit) flagged the 15-line duplication. Make the staticmethod delegate to the skill_deploy module function, matching the file's delegator pattern. R0801 now exits 0; ruff/format clean; 378 targeted tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sergio-sisternes-epam
pushed a commit
that referenced
this pull request
Jun 9, 2026
…e integrity) Sync with main through v0.19.0 (#1715, #1714, #1718, #1719, #1717). Conflicts resolved: - CHANGELOG.md: main cut the v0.19.0 release, moving prior Unreleased entries into a dated section. Took main's restructure; relocated the #1681 "Tightened Stage 2 code-complexity thresholds" entry to the new ## [Unreleased] section (released sections stay immutable per Keep a Changelog). Took main's #1702/#1710 close-ref for the lockfile entry. - src/apm_cli/install/services.py: #1718 added per-file skill-bundle integrity recording inline at a site my refactor had extracted into _log_skill_result. Kept the new _skill_bundle_file_entries helper in services.py (sibling of _deployed_path_entry); dropped main's duplicate inline _log_hook_display_payloads (already relocated to services_integrate.py and re-exported in my #1700 port); accepted the _log_skill_result delegator at the call site. - src/apm_cli/install/services_integrate.py: ported #1718's deployed.extend(_skill_bundle_file_entries(tp, ...)) into _log_skill_result's deployed-path loop (deployed aliases result["deployed_files"]), with a lazy import of the helper. Shadow gate: no markers, all src <=800 lines, ruff check + format clean, pylint R0801 EXIT=0, auth-signals clean; 11144 tests pass across the install/integration suites plus the #1718, #1700, and #1709 targeted sets. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Plugin-manifest skill collections currently copy every manifest skill into .apm/skills and then promote the entire standalone set, so --skill selections are lost outside the SKILL_BUNDLE path. This threads the normalized skill subset into standalone promotion, accepts both leaf names and nested manifest paths, adds a regression test for plugin-manifest collections, and updates the CLI docs for plugin subset selection.
How-to-test:
Closes #1707