Uh oh!
There was an error while loading. Please reload this page.
Fix skill deployment to .claude/skills/ and .opencode/skills/ for transitive deps - #448
Fix skill deployment to .claude/skills/ and .opencode/skills/ for transitive deps#448Daniel Meppiel (danielmeppiel) with Copilot wants to merge 3 commits into
Conversation
Co-authored-by: danielmeppiel <51440732+danielmeppiel@users.noreply.github.com> Agent-Logs-Url: https://github.com/microsoft/apm/sessions/b1decace-6986-439f-a988-d0fbfef588ab
…nsitive deps Pass integrate_claude/integrate_opencode flags from install to skill integrator so skills deploy to secondary targets even when the target directories do not yet exist. Also add missing .opencode/skills/ deployment to _integrate_native_skill. Co-authored-by: danielmeppiel <51440732+danielmeppiel@users.noreply.github.com> Agent-Logs-Url: https://github.com/microsoft/apm/sessions/b1decace-6986-439f-a988-d0fbfef588ab
There was a problem hiding this comment.
Pull request overview
Fixes skill deployment so skills from third-party/transitive dependencies are deployed to Claude and OpenCode targets when those targets are requested (not only when the directories already exist).
Changes:
- Add
integrate_claude/integrate_opencodeflags through the skill integration pipeline and use them to allow flag-driven deployment. - Add
.opencode/skills/deployment (including sub-skill promotion) for native skills. - Add/extend unit tests covering OpenCode skill deployment and flag behavior.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/apm_cli/integration/skill_integrator.py | Adds Claude/OpenCode flag parameters, extends deployment logic to .opencode/skills/, and enables deployment when flags are set even if target dirs do not pre-exist. |
src/apm_cli/commands/install.py | Plumbs integrate_claude / integrate_opencode into the skill integrator call. |
tests/unit/integration/test_skill_integrator.py | Adds new unit tests for OpenCode skill deployment and flag-driven directory creation/promotion behavior. |
CHANGELOG.md | Adds an Unreleased "Fixed" entry describing the corrected skill deployment behavior. |
Comments suppressed due to low confidence (1)
src/apm_cli/integration/skill_integrator.py:654
- The opening line of this docstring still says native skills are copied "optionally .claude/skills/ and .cursor/skills/", but the function now also supports
.opencode/skills/(and has integrate_opencode). Please update the first sentence so it matches the current behavior.
"""Copy a native Skill (with existing SKILL.md) to .github/skills/ and optionally .claude/skills/ and .cursor/skills/.
| integrate_claude=True, integrate_opencode=True, | ||
| ) | ||
| target_strs = [str(p) for p in result.target_paths] |
There was a problem hiding this comment.
These assertions convert Paths to str() and then look for POSIX-style substrings (".github/skills/..." etc). On Windows, str(Path) uses backslashes, so this test will fail. Use p.as_posix() (or replace('\','/')) for platform-independent matching, or compare Paths/relative Paths directly.
| target_strs= [str(p) forpinresult.target_paths] | |
| target_strs= [p.as_posix() forpinresult.target_paths] |
| Args: | ||
| package_info: PackageInfo object with package metadata. | ||
| project_root: Root directory of the project. | ||
| integrate_claude: When True, deploy to .claude/skills/ even if | ||
| .claude/ does not yet exist. | ||
| integrate_opencode: When True, deploy to .opencode/skills/ even if | ||
| .opencode/ does not yet exist. |
There was a problem hiding this comment.
The method-level docstring above still describes promotion to .github/skills/ and only .claude/skills/ "when present", but the code also promotes to .cursor/skills/ and .opencode/skills/ and can create .claude/.opencode when integrate_* flags are true. Please update the docstring text so it matches the actual target behavior.
This issue also appears on line 653 of the same file.
See below for a potential fix:
``.apm/skills/``. This method promotes them to ``.github/skills/``
and also:
- ``.claude/skills/`` when ``integrate_claude`` is True or a
``.claude/`` directory already exists (creating it if needed)
- ``.cursor/skills/`` when a ``.cursor/`` directory exists
- ``.opencode/skills/`` when ``integrate_opencode`` is True or an
``.opencode/`` directory already exists (creating it if needed)
This promotion does not create a top-level skill entry for the
parent package.
Args:
package_info: PackageInfo object with package metadata.
project_root: Root directory of the project.
integrate_claude: When True, deploy to .claude/skills/ even if
.claude/ does not yet exist (the directory may be created).
integrate_opencode: When True, deploy to .opencode/skills/ even if
.opencode/ does not yet exist (the directory may be created).
| integrate_claude: When True, deploy to .claude/skills/ even if | ||
| .claude/ does not yet exist. | ||
| integrate_opencode: When True, deploy to .opencode/skills/ even if | ||
| .opencode/ does not yet exist. |
There was a problem hiding this comment.
Behavior change: with integrate_claude/integrate_opencode True, this integrator will now deploy (and create) .claude/skills/ / .opencode/skills/ even when those root dirs do not already exist. Several docs pages currently state OpenCode/Claude skill deployment happens only when .opencode//.claude/ already exists (e.g., docs/src/content/docs/guides/skills.md and integrations/ide-tool-integration.md). Please update the docs to reflect the new flag-based behavior (and how users enable it via target: claude|opencode|all).
| ### Fixed | ||
| - Skills from third-party and transitive dependencies now deploy to `.claude/skills/` and `.opencode/skills/` when the install target includes those platforms (`target: all`, `target: claude`, `target: opencode`) |
There was a problem hiding this comment.
Changelog entry format: per Keep a Changelog/SemVer conventions in this repo, each bullet should be a single concise line ending with the PR number like (#123). This new entry is missing the (#PR_NUMBER) suffix (and should keep to one line per PR).
Daniel Meppiel (danielmeppiel)
commented
Mar 26, 2026
Closing as supersededThis bug is already fixed on main via the target registry consolidation in PR #456 (merged). What changed: All three skill deployment functions now use an
This means Why the flag-based approach from this PR wouldn't merge cleanly: PR #456 eliminated hardcoded target awareness from the install command's call chain. Re-introducing Thank you to copilot-swe-agent for identifying the bug -- the analysis was correct, the fix was just delivered through a different architectural approach. |
Description
Skills from third-party and transitive dependencies only deploy to
.github/skills/, missing.claude/skills/and.opencode/skills/even when the install target includes those platforms.Two root causes:
_integrate_native_skill()had no.opencode/skills/deployment block at allintegrate_claude/integrate_opencodeflags from target detection were never passed to the skill integrator — it relied solely on directory pre-existence checks, but.claude//.opencode/may not exist whenapm installrunsChanges
skill_integrator.py: Addintegrate_claude/integrate_opencodeparams tointegrate_package_skill(),_integrate_native_skill(), and_promote_sub_skills_standalone(). Deploy to target when flag isTrueOR directory already exists. Add.opencode/skills/deployment block with sub-skill promotion.install.py: Passintegrate_claude/integrate_opencodefrom_integrate_package_primitives()to the skill integrator call.Default flag values are
False, preserving existing behavior for callers that don't pass them.Type of change
Testing
11 new unit tests covering:
.opencode/skills/deployment, flag-based directory creation for Claude and OpenCode, backward compatibility with default flags, sub-skill promotion with flags.💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.