fix: use effective_root for skill deployment at user scope by sergio-sisternes-epam · Pull Request #537 · microsoft/apm

sergio-sisternes-epam · 2026-04-02T10:39:15Z

Description

When running apm install -g --target opencode, skills were silently deployed to ~/.opencode/skills/ instead of the correct user-scope path ~/.config/opencode/skills/. No error or warning was raised.

Root cause: All integrators used target.root_dir directly instead of target.effective_root(user_scope=True). This affects any target where user_root_dir differs from root_dir (Copilot: .github vs .copilot, OpenCode: .opencode vs .config/opencode).

Changes:

Thread user_scope: bool = False parameter through ALL integrator *_for_target() and sync_for_target() methods
Replace target.root_dir with target.effective_root(user_scope=user_scope) in deploy paths and auto_create guards across all 5 integrators
Thread user_scope from install.py dispatch loop and uninstall engine
Fix hardcoded .github in skill sub-skill counting
11 new tests for user-scope skill deployment

Files changed:

src/apm_cli/integration/skill_integrator.py — skill deploy paths + auto_create guard
src/apm_cli/integration/agent_integrator.py — agent deploy paths + auto_create guard + sync
src/apm_cli/integration/command_integrator.py — command deploy paths + auto_create guard + sync
src/apm_cli/integration/instruction_integrator.py — instruction deploy paths + auto_create guard + sync
src/apm_cli/integration/prompt_integrator.py — prompt auto_create guard + sync
src/apm_cli/commands/install.py — thread user_scope to integrator dispatch
src/apm_cli/commands/uninstall/engine.py — thread user_scope to sync calls

Fixes #530

PR fix: skip unsupported Copilot CLI primitives at user scope #536 — Copilot user-scope unsupported primitives (instructions, prompts)
Zero remaining target.root_dir references in integrator deploy/guard logic

Type of change

Bug fix
New feature
Documentation
Maintenance / refactor

Testing

Tested locally
All existing tests pass (3,410 unit tests)
Added tests for new functionality (11 new tests)

When running apm install -g --target opencode, skills were deployed to ~/.opencode/skills/ instead of ~/.config/opencode/skills/. - Thread user_scope through skill integrator methods - Replace target.root_dir with target.effective_root(user_scope) - Add auto_create guard matching other integrators - Fix hardcoded .github in sub-skill counting Fixes microsoft#530 Co-authored-by: Copilot <[email protected]>

Copilot

Pull request overview

Fixes user-scope skill deployment so apm install -g --target opencode installs skills under the correct user-scope root (e.g. ~/.config/opencode/...) and applies the same auto_create opt-in behavior used by other integrators.

Changes:

Thread user_scope through skill integration entrypoints and switch deployment roots to target.effective_root(user_scope=...).
Add auto_create guard to skill deployment/promotion paths to avoid creating target roots unexpectedly.
Add unit tests covering user-scope vs project-scope root selection and auto_create behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File	Description
`src/apm_cli/integration/skill_integrator.py`	Adds `user_scope` plumbing, uses `effective_root()` for skill paths, and adds `auto_create` guards / primary-root logic.
`src/apm_cli/commands/install.py`	Passes `_user_scope` into `integrate_package_skill()` during install dispatch.
`tests/unit/integration/test_skill_integrator.py`	Adds coverage for user-scope deployment, `auto_create` guard behavior, and primary-root counting.

Comments suppressed due to low confidence (1)

src/apm_cli/commands/install.py:1099

For user-scope roots that contain nested paths (e.g. ".config/opencode"), _skill_target_dirs.add(rel.parts[0]) collapses the logged deploy root to just ".config", so the integration message can point at ".config/skills/" even though the real path is ".config/opencode/skills/". Consider deriving the displayed root as the path prefix up to the "skills" component (or using the target profile's effective_root) to keep logs accurate.

    skill_result = skill_integrator.integrate_package_skill(
        package_info, project_root,
        diagnostics=diagnostics, managed_files=managed_files, force=force,
        targets=targets, user_scope=_user_scope,
    )
    _skill_target_dirs: set[str] = builtins.set()
    for tp in skill_result.target_paths:
        rel = tp.relative_to(project_root)
        if rel.parts:
            _skill_target_dirs.add(rel.parts[0])
    _skill_targets = sorted(_skill_target_dirs)
    _skill_target_str = ", ".join(f"{d}/skills/" for d in _skill_targets) or "skills/"

Apply the same fix from skill_integrator to all remaining integrators: agent, command, instruction, and prompt integrators now use target.effective_root(user_scope) instead of target.root_dir for deploy paths and auto_create guards. Also threads user_scope from install.py dispatch loop and uninstall engine to all integrator *_for_target() and sync_for_target() methods. Related: microsoft#536 (Copilot user-scope primitives) Co-authored-by: Copilot <[email protected]>

…rator - Use primary_assigned flag instead of loop index for is_primary in both _promote_sub_skills_standalone() and _integrate_native_skill(), so the first target that passes the auto_create guard becomes primary - Return skill_skipped=True when all targets are skipped by auto_create - Replace non-ASCII em dashes with ASCII in test comments Co-authored-by: Copilot <[email protected]>

Co-authored-by: Copilot <[email protected]>

Introduce TargetProfile.for_scope() and resolve_targets() so scope resolution happens exactly once -- no parameter threading, no caller responsibility. This supersedes PRs #536 and #537 and folds in #527. Scope resolution: - for_scope(user_scope) returns a frozen copy with root_dir resolved and unsupported primitives filtered out - resolve_targets() is the single entry point combining detection, scope resolution, and primitive filtering - Integrators read target.root_dir directly -- always correct - partition_managed_files() uses longest-prefix-match routing, fixing .config/opencode/ multi-level root paths - validate_deploy_path() derives prefixes from resolved targets - Uninstall Phase 2 re-integration uses resolved targets, eliminating wrong-path deployment bug Claude Code instructions (#527): - Deploy .instructions.md to .claude/rules/*.md with applyTo: to paths: frontmatter conversion - Legacy glob disabled for .claude/rules/ to protect user .md files - 22 new tests for conversion, integration, sync, and partition Copilot CLI user-scope fix (#536): - Add instructions to unsupported_user_primitives (per official docs) - Fix resolve_dependencies to use apm_dir at user scope Fixes #530 Co-authored-by: Copilot <[email protected]>

danielmeppiel · 2026-04-02T13:17:09Z

Superseded by #542 which implements a cleaner architecture: TargetProfile.for_scope() resolves scope once on the data object, so integrators never need user_scope parameters. Your auto_create guard and primary_assigned pattern for skills are addressed through the resolved targets approach. Thank you for the contribution!

danielmeppiel · 2026-04-02T13:17:18Z

Closing in favor of #542 (scope-resolved target profiles architecture).

Introduce TargetProfile.for_scope() and resolve_targets() so scope resolution happens exactly once -- no parameter threading, no caller responsibility. This supersedes PRs #536 and #537 and folds in #527. Scope resolution: - for_scope(user_scope) returns a frozen copy with root_dir resolved and unsupported primitives filtered out - resolve_targets() is the single entry point combining detection, scope resolution, and primitive filtering - Integrators read target.root_dir directly -- always correct - partition_managed_files() uses longest-prefix-match routing, fixing .config/opencode/ multi-level root paths - validate_deploy_path() derives prefixes from resolved targets - Uninstall Phase 2 re-integration uses resolved targets, eliminating wrong-path deployment bug Claude Code instructions (#527): - Deploy .instructions.md to .claude/rules/*.md with applyTo: to paths: frontmatter conversion - Legacy glob disabled for .claude/rules/ to protect user .md files - 22 new tests for conversion, integration, sync, and partition Copilot CLI user-scope fix (#536): - Add instructions to unsupported_user_primitives (per official docs) - Fix resolve_dependencies to use apm_dir at user scope Fixes #530 Co-authored-by: Copilot <[email protected]>

* fix: scope-resolved target profiles + Claude Code instructions Introduce TargetProfile.for_scope() and resolve_targets() so scope resolution happens exactly once -- no parameter threading, no caller responsibility. This supersedes PRs #536 and #537 and folds in #527. Scope resolution: - for_scope(user_scope) returns a frozen copy with root_dir resolved and unsupported primitives filtered out - resolve_targets() is the single entry point combining detection, scope resolution, and primitive filtering - Integrators read target.root_dir directly -- always correct - partition_managed_files() uses longest-prefix-match routing, fixing .config/opencode/ multi-level root paths - validate_deploy_path() derives prefixes from resolved targets - Uninstall Phase 2 re-integration uses resolved targets, eliminating wrong-path deployment bug Claude Code instructions (#527): - Deploy .instructions.md to .claude/rules/*.md with applyTo: to paths: frontmatter conversion - Legacy glob disabled for .claude/rules/ to protect user .md files - 22 new tests for conversion, integration, sync, and partition Copilot CLI user-scope fix (#536): - Add instructions to unsupported_user_primitives (per official docs) - Fix resolve_dependencies to use apm_dir at user scope Fixes #530 Co-authored-by: Copilot <[email protected]> * fix: address PR review -- trie routing, legacy prefix cleanup, CHANGELOG refs - Replace O(M*P) sorted-prefix scan with O(depth) trie lookup in partition_managed_files (reviewer comment 1) - Use default KNOWN_TARGETS for uninstall partition so legacy deployed_files from buggy user-scope installs are still bucketed and cleaned up (reviewer comment 2) - Remove targets= from orphan validate_deploy_path so legacy .github/ paths from old user-scope installs can be cleaned up (reviewer comment 3) - Fix CHANGELOG PR references to use #542 (reviewer comment 4) Co-authored-by: Copilot <[email protected]> * fix: dynamic skill sync prefixes + uninstall Phase 2 targets Fix #538: pass resolved targets to skill_integrator.integrate_package_skill() in uninstall Phase 2 so user-scope re-integration deploys to .copilot/skills/ not .github/skills/. Add auto_create guard to copy_skill_to_target(). Fix #539: refactor sync_integration() to derive skill prefixes dynamically from targets instead of hardcoded .github/skills/, .claude/skills/, etc. User-scope paths like .copilot/skills/ and .config/opencode/skills/ are now handled correctly. Pass resolved targets from uninstall engine. 9 new tests covering resolved-target routing, auto_create guards, user-scope manifest removal, legacy cleanup, and cross-tool guards. Co-authored-by: Copilot <[email protected]> --------- Co-authored-by: Copilot <[email protected]>

Copilot AI review requested due to automatic review settings April 2, 2026 10:39

sergio-sisternes-epam requested a review from danielmeppiel as a code owner April 2, 2026 10:39

Copilot started reviewing on behalf of sergio-sisternes-epam April 2, 2026 10:39 View session

Copilot AI reviewed Apr 2, 2026

View reviewed changes

Sergio Sisternes and others added 2 commits April 2, 2026 12:51

This was referenced Apr 2, 2026

[BUG] copy_skill_to_target falls back to project-scope active_targets when targets=None at user scope #538

Closed

[BUG] sync_integration() hard-codes skill prefixes -- misses user-scope paths during uninstall #539

Closed

docs: add CHANGELOG entry for user-scope deploy path fix

214c9bf

Co-authored-by: Copilot <[email protected]>

danielmeppiel mentioned this pull request Apr 2, 2026

fix: scope-resolved target profiles + Claude Code instructions #542

Merged

6 tasks

danielmeppiel closed this Apr 2, 2026

sergio-sisternes-epam deleted the fix/opencode-skill-user-scope branch April 3, 2026 18:16

sergio-sisternes-epam mentioned this pull request Apr 3, 2026

[BUG] Global hook deployment writes to ~/.github/hooks/ instead of ~/.copilot/hooks/ #565

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: use effective_root for skill deployment at user scope#537

fix: use effective_root for skill deployment at user scope#537
sergio-sisternes-epam wants to merge 4 commits intomicrosoft:mainfrom
sergio-sisternes-epam:fix/opencode-skill-user-scope

sergio-sisternes-epam commented Apr 2, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

danielmeppiel commented Apr 2, 2026

Uh oh!

danielmeppiel commented Apr 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

sergio-sisternes-epam commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

danielmeppiel commented Apr 2, 2026

Uh oh!

danielmeppiel commented Apr 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sergio-sisternes-epam commented Apr 2, 2026 •

edited

Loading