From 9e17613a59b04ae230342affb1ebf7ab23c7e0e8 Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Thu, 2 Apr 2026 12:38:53 +0200 Subject: [PATCH 1/4] fix: use effective_root for skill deployment at user scope 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 #530 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/commands/install.py | 2 +- src/apm_cli/integration/skill_integrator.py | 43 ++- .../unit/integration/test_skill_integrator.py | 309 ++++++++++++++++++ 3 files changed, 343 insertions(+), 11 deletions(-) diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 06b562aca..59b0d3421 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -1088,7 +1088,7 @@ def _log_integration(msg): skill_result = skill_integrator.integrate_package_skill( package_info, project_root, diagnostics=diagnostics, managed_files=managed_files, force=force, - targets=targets, + targets=targets, user_scope=_user_scope, ) _skill_target_dirs: set[str] = builtins.set() for tp in skill_result.target_paths: diff --git a/src/apm_cli/integration/skill_integrator.py b/src/apm_cli/integration/skill_integrator.py index d09664fc3..bf0df8555 100644 --- a/src/apm_cli/integration/skill_integrator.py +++ b/src/apm_cli/integration/skill_integrator.py @@ -248,6 +248,7 @@ def copy_skill_to_target( source_path: Path, target_base: Path, targets=None, + user_scope: bool = False, ) -> list[Path]: """Copy skill directory to all active target skills/ directories. @@ -308,7 +309,13 @@ def copy_skill_to_target( if not target.supports("skills"): continue skills_mapping = target.primitives["skills"] - effective_root = skills_mapping.deploy_root or target.root_dir + effective_root = skills_mapping.deploy_root or target.effective_root(user_scope=user_scope) + + # Skip if target dir doesn't exist and auto_create is disabled + target_root_dir = target_base / target.effective_root(user_scope=user_scope) + if not target.auto_create and not target_root_dir.is_dir(): + continue + skill_dir = target_base / effective_root / "skills" / skill_name skill_dir.parent.mkdir(parents=True, exist_ok=True) if skill_dir.exists(): @@ -574,7 +581,7 @@ def _build_skill_ownership_map(project_root: Path) -> dict[str, str]: def _promote_sub_skills_standalone( self, package_info, project_root: Path, diagnostics=None, managed_files=None, force: bool = False, logger=None, - targets=None, + targets=None, user_scope: bool = False, ) -> tuple[int, list[Path]]: """Promote sub-skills from a package that is NOT itself a skill. @@ -611,7 +618,13 @@ def _promote_sub_skills_standalone( is_primary = (idx == 0) # first active target owns diagnostics skills_mapping = target.primitives["skills"] - effective_root = skills_mapping.deploy_root or target.root_dir + effective_root = skills_mapping.deploy_root or target.effective_root(user_scope=user_scope) + + # Skip if target dir doesn't exist and auto_create is disabled + target_root_dir = project_root / target.effective_root(user_scope=user_scope) + if not target.auto_create and not target_root_dir.is_dir(): + continue + target_skills_root = project_root / effective_root / "skills" target_skills_root.mkdir(parents=True, exist_ok=True) @@ -633,7 +646,7 @@ def _promote_sub_skills_standalone( def _integrate_native_skill( self, package_info, project_root: Path, source_skill_md: Path, diagnostics=None, managed_files=None, force: bool = False, - logger=None, targets=None, + logger=None, targets=None, user_scope: bool = False, ) -> SkillIntegrationResult: """Copy a native Skill (with existing SKILL.md) to all active targets. @@ -707,6 +720,7 @@ def _integrate_native_skill( owned_by = self._build_skill_ownership_map(project_root) sub_skills_dir = package_path / ".apm" / "skills" + primary_effective_root = None for idx, target in enumerate(targets): if not target.supports("skills"): @@ -714,7 +728,16 @@ def _integrate_native_skill( is_primary = (idx == 0) # first active target owns diagnostics skills_mapping = target.primitives["skills"] - effective_root = skills_mapping.deploy_root or target.root_dir + effective_root = skills_mapping.deploy_root or target.effective_root(user_scope=user_scope) + + # Skip if target dir doesn't exist and auto_create is disabled + target_root_dir = project_root / target.effective_root(user_scope=user_scope) + if not target.auto_create and not target_root_dir.is_dir(): + continue + + if primary_effective_root is None: + primary_effective_root = effective_root + target_skill_dir = project_root / effective_root / "skills" / skill_name if is_primary: @@ -748,7 +771,7 @@ def _integrate_native_skill( all_target_paths.extend(sub_deployed) # Count unique sub-skills from primary target only - primary_root = project_root / ".github" / "skills" + primary_root = project_root / (primary_effective_root or ".github") / "skills" sub_skills_count = sum( 1 for p in all_target_paths if p.parent == primary_root and p.name != skill_name @@ -765,7 +788,7 @@ def _integrate_native_skill( target_paths=all_target_paths ) - def integrate_package_skill(self, package_info, project_root: Path, diagnostics=None, managed_files=None, force: bool = False, logger=None, targets=None) -> SkillIntegrationResult: + def integrate_package_skill(self, package_info, project_root: Path, diagnostics=None, managed_files=None, force: bool = False, logger=None, targets=None, user_scope: bool = False) -> SkillIntegrationResult: """Integrate a package's skill into all active target directories. Copies native skills (packages with SKILL.md at root) to every active @@ -793,7 +816,7 @@ def integrate_package_skill(self, package_info, project_root: Path, diagnostics= # Even non-skill packages may ship sub-skills under .apm/skills/. # Promote them so Copilot can discover them independently. sub_skills_count, sub_deployed = self._promote_sub_skills_standalone( - package_info, project_root, diagnostics=diagnostics, managed_files=managed_files, force=force, logger=logger, targets=targets + package_info, project_root, diagnostics=diagnostics, managed_files=managed_files, force=force, logger=logger, targets=targets, user_scope=user_scope ) return SkillIntegrationResult( skill_created=False, @@ -826,12 +849,12 @@ def integrate_package_skill(self, package_info, project_root: Path, diagnostics= # Check if this is a native Skill (already has SKILL.md at root) source_skill_md = package_path / "SKILL.md" if source_skill_md.exists(): - return self._integrate_native_skill(package_info, project_root, source_skill_md, diagnostics=diagnostics, managed_files=managed_files, force=force, logger=logger, targets=targets) + return self._integrate_native_skill(package_info, project_root, source_skill_md, diagnostics=diagnostics, managed_files=managed_files, force=force, logger=logger, targets=targets, user_scope=user_scope) # No SKILL.md at root -- not a skill package. # Still promote any sub-skills shipped under .apm/skills/. sub_skills_count, sub_deployed = self._promote_sub_skills_standalone( - package_info, project_root, diagnostics=diagnostics, managed_files=managed_files, force=force, logger=logger, targets=targets + package_info, project_root, diagnostics=diagnostics, managed_files=managed_files, force=force, logger=logger, targets=targets, user_scope=user_scope ) return SkillIntegrationResult( skill_created=False, diff --git a/tests/unit/integration/test_skill_integrator.py b/tests/unit/integration/test_skill_integrator.py index fbf62d603..52b03f0b7 100644 --- a/tests/unit/integration/test_skill_integrator.py +++ b/tests/unit/integration/test_skill_integrator.py @@ -3093,3 +3093,312 @@ def test_other_targets_still_deploy_to_own_root(self): assert len(deployed) == 1 assert ".github" in str(deployed[0]) assert (self.root / ".github" / "skills" / "my-skill" / "SKILL.md").exists() + + +class TestUserScopeSkillIntegration: + """Tests for user_scope=True and auto_create guard in skill integration. + + Issue #530: skills deploy to wrong paths at user scope. + OpenCode config: root_dir=".opencode", user_root_dir=".config/opencode", + auto_create=False. + """ + + def setup_method(self): + self.temp_dir = tempfile.mkdtemp() + self.project_root = Path(self.temp_dir) + self.apm_modules = self.project_root / "apm_modules" + self.apm_modules.mkdir(parents=True) + self.integrator = SkillIntegrator() + + def teardown_method(self): + shutil.rmtree(self.temp_dir, ignore_errors=True) + + def _create_package_info( + self, + name: str = "test-skill", + version: str = "1.0.0", + commit: str = "abc123", + install_path: Path = None, + source: str = None, + dependency_ref: DependencyReference = None, + package_type: PackageType = PackageType.CLAUDE_SKILL, + ) -> PackageInfo: + package = APMPackage( + name=name, + version=version, + package_path=install_path or self.project_root / "package", + source=source or f"github.com/test/{name}", + ) + resolved_ref = ResolvedReference( + original_ref="main", + ref_type=GitReferenceType.BRANCH, + resolved_commit=commit, + ref_name="main", + ) + return PackageInfo( + package=package, + install_path=install_path or self.project_root / "package", + resolved_reference=resolved_ref, + installed_at=datetime.now().isoformat(), + dependency_ref=dependency_ref, + package_type=package_type, + ) + + # ========== integrate_package_skill — user_scope path selection ========== + + def test_user_scope_deploys_to_user_root_dir(self): + """With user_scope=True, skills deploy to user_root_dir (.config/opencode).""" + from apm_cli.integration.targets import KNOWN_TARGETS + + opencode = KNOWN_TARGETS["opencode"] + # Create the user-scope root so auto_create guard passes + (self.project_root / opencode.user_root_dir).mkdir(parents=True) + + skill_source = self.apm_modules / "owner" / "my-skill" + skill_source.mkdir(parents=True) + (skill_source / "SKILL.md").write_text("---\nname: my-skill\n---\n# My Skill") + + pkg = self._create_package_info(name="my-skill", install_path=skill_source) + result = self.integrator.integrate_package_skill( + pkg, self.project_root, targets=[opencode], user_scope=True, + ) + + assert result.skill_created is True + expected = self.project_root / ".config" / "opencode" / "skills" / "my-skill" / "SKILL.md" + assert expected.exists() + # Must NOT deploy to .opencode/skills/ + assert not (self.project_root / ".opencode" / "skills").exists() + + def test_project_scope_deploys_to_root_dir(self): + """With user_scope=False (default), skills deploy to root_dir (.opencode).""" + from apm_cli.integration.targets import KNOWN_TARGETS + + opencode = KNOWN_TARGETS["opencode"] + # Create project-scope root + (self.project_root / opencode.root_dir).mkdir(parents=True) + + skill_source = self.apm_modules / "owner" / "my-skill" + skill_source.mkdir(parents=True) + (skill_source / "SKILL.md").write_text("---\nname: my-skill\n---\n# My Skill") + + pkg = self._create_package_info(name="my-skill", install_path=skill_source) + result = self.integrator.integrate_package_skill( + pkg, self.project_root, targets=[opencode], user_scope=False, + ) + + assert result.skill_created is True + expected = self.project_root / ".opencode" / "skills" / "my-skill" / "SKILL.md" + assert expected.exists() + + # ========== auto_create guard — no deployment when dir missing ========== + + def test_auto_create_false_skips_when_dir_missing(self): + """auto_create=False targets are skipped when their root dir doesn't exist.""" + from apm_cli.integration.targets import KNOWN_TARGETS + + opencode = KNOWN_TARGETS["opencode"] + assert opencode.auto_create is False # precondition + + # Do NOT create .opencode/ or .config/opencode/ + skill_source = self.apm_modules / "owner" / "my-skill" + skill_source.mkdir(parents=True) + (skill_source / "SKILL.md").write_text("---\nname: my-skill\n---\n# My Skill") + + pkg = self._create_package_info(name="my-skill", install_path=skill_source) + result = self.integrator.integrate_package_skill( + pkg, self.project_root, targets=[opencode], user_scope=False, + ) + + # Skipped entirely — nothing created + assert len(result.target_paths) == 0 + assert not (self.project_root / ".opencode" / "skills").exists() + + def test_auto_create_false_skips_user_scope_when_user_dir_missing(self): + """auto_create=False with user_scope=True skips when user_root_dir missing.""" + from apm_cli.integration.targets import KNOWN_TARGETS + + opencode = KNOWN_TARGETS["opencode"] + # Create .opencode/ (project scope) but NOT .config/opencode/ (user scope) + (self.project_root / ".opencode").mkdir() + + skill_source = self.apm_modules / "owner" / "my-skill" + skill_source.mkdir(parents=True) + (skill_source / "SKILL.md").write_text("---\nname: my-skill\n---\n# My Skill") + + pkg = self._create_package_info(name="my-skill", install_path=skill_source) + result = self.integrator.integrate_package_skill( + pkg, self.project_root, targets=[opencode], user_scope=True, + ) + + assert len(result.target_paths) == 0 + assert not (self.project_root / ".config" / "opencode" / "skills").exists() + + def test_auto_create_true_creates_dir(self): + """auto_create=True targets (copilot) create their dir even if absent.""" + from apm_cli.integration.targets import KNOWN_TARGETS + + copilot = KNOWN_TARGETS["copilot"] + assert copilot.auto_create is True # precondition + assert not (self.project_root / ".github").exists() + + skill_source = self.apm_modules / "owner" / "my-skill" + skill_source.mkdir(parents=True) + (skill_source / "SKILL.md").write_text("---\nname: my-skill\n---\n# My Skill") + + pkg = self._create_package_info(name="my-skill", install_path=skill_source) + result = self.integrator.integrate_package_skill( + pkg, self.project_root, targets=[copilot], user_scope=False, + ) + + assert result.skill_created is True + assert (self.project_root / ".github" / "skills" / "my-skill" / "SKILL.md").exists() + + # ========== _promote_sub_skills_standalone — user_scope ========== + + def test_sub_skill_promotion_uses_user_root(self): + """Sub-skills promoted at user_scope use user_root_dir.""" + from apm_cli.integration.targets import KNOWN_TARGETS + + opencode = KNOWN_TARGETS["opencode"] + (self.project_root / opencode.user_root_dir).mkdir(parents=True) + + package_dir = self.project_root / "my-pkg" + package_dir.mkdir() + (package_dir / "SKILL.md").write_text("---\nname: my-pkg\n---\n# Pkg") + sub_dir = package_dir / ".apm" / "skills" / "sub-a" + sub_dir.mkdir(parents=True) + (sub_dir / "SKILL.md").write_text("---\nname: sub-a\n---\n# Sub-A") + + pkg = self._create_package_info(name="my-pkg", install_path=package_dir) + result = self.integrator.integrate_package_skill( + pkg, self.project_root, targets=[opencode], user_scope=True, + ) + + expected = self.project_root / ".config" / "opencode" / "skills" / "sub-a" / "SKILL.md" + assert expected.exists() + assert not (self.project_root / ".opencode" / "skills").exists() + + def test_sub_skill_standalone_skips_auto_create_false(self): + """Standalone sub-skill promotion skips auto_create=False targets with no dir.""" + from apm_cli.integration.targets import KNOWN_TARGETS + + opencode = KNOWN_TARGETS["opencode"] + # Don't create .opencode/ or .config/opencode/ + + # Package typed as INSTRUCTIONS so it goes through _promote_sub_skills_standalone + package_dir = self.project_root / "my-instructions" + package_dir.mkdir() + sub_dir = package_dir / ".apm" / "skills" / "sub-a" + sub_dir.mkdir(parents=True) + (sub_dir / "SKILL.md").write_text("---\nname: sub-a\n---\n# Sub-A") + + pkg = self._create_package_info( + name="my-instructions", install_path=package_dir, + package_type=PackageType.APM_PACKAGE, + ) + result = self.integrator.integrate_package_skill( + pkg, self.project_root, targets=[opencode], user_scope=False, + ) + + assert result.skill_skipped is True + assert len(result.target_paths) == 0 + assert not (self.project_root / ".opencode" / "skills").exists() + + # ========== copy_skill_to_target — user_scope ========== + + def test_copy_skill_to_target_user_scope(self): + """copy_skill_to_target with user_scope=True uses user_root_dir.""" + from apm_cli.integration.targets import KNOWN_TARGETS + + opencode = KNOWN_TARGETS["opencode"] + (self.project_root / opencode.user_root_dir).mkdir(parents=True) + + skill_source = self.apm_modules / "owner" / "my-skill" + skill_source.mkdir(parents=True) + (skill_source / "SKILL.md").write_text("---\nname: my-skill\n---\n# My Skill") + + pi = Mock() + pi.install_path = skill_source + pi.package = Mock() + pi.package.name = "my-skill" + pi.package_type = PackageType.CLAUDE_SKILL + + deployed = copy_skill_to_target( + pi, skill_source, self.project_root, + targets=[opencode], user_scope=True, + ) + + assert len(deployed) == 1 + assert ".config" in str(deployed[0]) + expected = self.project_root / ".config" / "opencode" / "skills" / "my-skill" / "SKILL.md" + assert expected.exists() + assert not (self.project_root / ".opencode" / "skills").exists() + + def test_copy_skill_to_target_auto_create_guard(self): + """copy_skill_to_target skips auto_create=False targets with no dir.""" + from apm_cli.integration.targets import KNOWN_TARGETS + + opencode = KNOWN_TARGETS["opencode"] + # Do NOT create .opencode/ + + skill_source = self.apm_modules / "owner" / "my-skill" + skill_source.mkdir(parents=True) + (skill_source / "SKILL.md").write_text("---\nname: my-skill\n---\n# My Skill") + + pi = Mock() + pi.install_path = skill_source + pi.package = Mock() + pi.package.name = "my-skill" + pi.package_type = PackageType.CLAUDE_SKILL + + deployed = copy_skill_to_target( + pi, skill_source, self.project_root, targets=[opencode], + ) + + assert len(deployed) == 0 + assert not (self.project_root / ".opencode" / "skills").exists() + + # ========== primary_root — dynamic, not hardcoded .github ========== + + def test_primary_root_uses_first_active_target(self): + """Sub-skill counting uses effective root from first active target, not hardcoded .github.""" + from apm_cli.integration.targets import KNOWN_TARGETS + + opencode = KNOWN_TARGETS["opencode"] + (self.project_root / opencode.user_root_dir).mkdir(parents=True) + + package_dir = self.project_root / "my-pkg" + package_dir.mkdir() + (package_dir / "SKILL.md").write_text("---\nname: my-pkg\n---\n# Pkg") + sub_dir = package_dir / ".apm" / "skills" / "sub-a" + sub_dir.mkdir(parents=True) + (sub_dir / "SKILL.md").write_text("---\nname: sub-a\n---\n# Sub-A") + + pkg = self._create_package_info(name="my-pkg", install_path=package_dir) + result = self.integrator.integrate_package_skill( + pkg, self.project_root, targets=[opencode], user_scope=True, + ) + + # Sub-skill should be counted — it's under the primary target's root + assert result.sub_skills_promoted == 1 + + # ========== backward compatibility — user_scope default ========== + + def test_user_scope_defaults_to_false(self): + """Calling without user_scope uses project-scope (root_dir).""" + from apm_cli.integration.targets import KNOWN_TARGETS + + copilot = KNOWN_TARGETS["copilot"] + + skill_source = self.apm_modules / "owner" / "my-skill" + skill_source.mkdir(parents=True) + (skill_source / "SKILL.md").write_text("---\nname: my-skill\n---\n# My Skill") + + pkg = self._create_package_info(name="my-skill", install_path=skill_source) + # Call WITHOUT user_scope — should work exactly as before + result = self.integrator.integrate_package_skill( + pkg, self.project_root, targets=[copilot], + ) + + assert result.skill_created is True + assert (self.project_root / ".github" / "skills" / "my-skill" / "SKILL.md").exists() From 0966333b198ebcbbf284a8f0ed535499508b63b2 Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Thu, 2 Apr 2026 12:51:09 +0200 Subject: [PATCH 2/4] fix: thread effective_root(user_scope) through all integrators 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: #536 (Copilot user-scope primitives) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/commands/install.py | 2 +- src/apm_cli/commands/uninstall/engine.py | 4 ++-- src/apm_cli/integration/agent_integrator.py | 8 +++++--- src/apm_cli/integration/command_integrator.py | 8 +++++--- src/apm_cli/integration/instruction_integrator.py | 8 +++++--- src/apm_cli/integration/prompt_integrator.py | 6 ++++-- 6 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 59b0d3421..7795cea63 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -1065,7 +1065,7 @@ def _log_integration(msg): _int_result = getattr(_integrator, _method_name)( _target, package_info, project_root, force=force, managed_files=managed_files, - diagnostics=diagnostics, + diagnostics=diagnostics, user_scope=_user_scope, ) if _int_result.files_integrated > 0: result[_counter_key] += _int_result.files_integrated diff --git a/src/apm_cli/commands/uninstall/engine.py b/src/apm_cli/commands/uninstall/engine.py index 27a37ea8e..802f06eb1 100644 --- a/src/apm_cli/commands/uninstall/engine.py +++ b/src/apm_cli/commands/uninstall/engine.py @@ -224,7 +224,7 @@ def _cleanup_transitive_orphans(lockfile, packages_to_remove, apm_modules_dir, a return removed, actual_orphans -def _sync_integrations_after_uninstall(apm_package, project_root, all_deployed_files, logger): +def _sync_integrations_after_uninstall(apm_package, project_root, all_deployed_files, logger, user_scope: bool = False): """Remove deployed files and re-integrate from remaining packages.""" from ...integration.base_integrator import BaseIntegrator from ...models.apm_package import PackageInfo, validate_apm_package @@ -278,7 +278,7 @@ def _sync_integrations_after_uninstall(apm_package, project_root, all_deployed_f _managed_subset = _buckets.get(_bucket_key, set()) result = _integrator.sync_for_target( _target, apm_package, project_root, - managed_files=_managed_subset, + managed_files=_managed_subset, user_scope=user_scope, ) counts[_counter_key] += result.get("files_removed", 0) diff --git a/src/apm_cli/integration/agent_integrator.py b/src/apm_cli/integration/agent_integrator.py index d48a2b702..c55f806f5 100644 --- a/src/apm_cli/integration/agent_integrator.py +++ b/src/apm_cli/integration/agent_integrator.py @@ -99,6 +99,7 @@ def integrate_agents_for_target( force: bool = False, managed_files: set = None, diagnostics=None, + user_scope: bool = False, ) -> IntegrationResult: """Integrate agents from a package for a single *target*. @@ -110,9 +111,9 @@ def integrate_agents_for_target( if not mapping: return IntegrationResult(0, 0, 0, []) - effective_root = mapping.deploy_root or target.root_dir + effective_root = mapping.deploy_root or target.effective_root(user_scope=user_scope) target_root = project_root / effective_root - if not target.auto_create and not (project_root / target.root_dir).is_dir(): + if not target.auto_create and not (project_root / target.effective_root(user_scope=user_scope)).is_dir(): return IntegrationResult(0, 0, 0, []) self.init_link_resolver(package_info, project_root) @@ -165,12 +166,13 @@ def sync_for_target( apm_package, project_root: Path, managed_files: set = None, + user_scope: bool = False, ) -> Dict[str, int]: """Remove APM-managed agent files for a single *target*.""" mapping = target.primitives.get("agents") if not mapping: return {"files_removed": 0, "errors": 0} - effective_root = mapping.deploy_root or target.root_dir + effective_root = mapping.deploy_root or target.effective_root(user_scope=user_scope) prefix = f"{effective_root}/{mapping.subdir}/" legacy_dir = project_root / effective_root / mapping.subdir # Copilot uses .agent.md suffix; others use plain .md diff --git a/src/apm_cli/integration/command_integrator.py b/src/apm_cli/integration/command_integrator.py index 0795c584b..43576bbe4 100644 --- a/src/apm_cli/integration/command_integrator.py +++ b/src/apm_cli/integration/command_integrator.py @@ -119,6 +119,7 @@ def integrate_commands_for_target( force: bool = False, managed_files: set = None, diagnostics=None, + user_scope: bool = False, ) -> IntegrationResult: """Integrate prompt files as commands for a single *target*. @@ -130,9 +131,9 @@ def integrate_commands_for_target( if not mapping: return IntegrationResult(0, 0, 0, [], 0) - effective_root = mapping.deploy_root or target.root_dir + effective_root = mapping.deploy_root or target.effective_root(user_scope=user_scope) target_root = project_root / effective_root - if not target.auto_create and not (project_root / target.root_dir).is_dir(): + if not target.auto_create and not (project_root / target.effective_root(user_scope=user_scope)).is_dir(): return IntegrationResult(0, 0, 0, [], 0) prompt_files = self.find_prompt_files(package_info.install_path) @@ -185,12 +186,13 @@ def sync_for_target( apm_package, project_root: Path, managed_files: set = None, + user_scope: bool = False, ) -> Dict: """Remove APM-managed command files for a single *target*.""" mapping = target.primitives.get("commands") if not mapping: return {"files_removed": 0, "errors": 0} - effective_root = mapping.deploy_root or target.root_dir + effective_root = mapping.deploy_root or target.effective_root(user_scope=user_scope) prefix = f"{effective_root}/{mapping.subdir}/" legacy_dir = project_root / effective_root / mapping.subdir return self.sync_remove_files( diff --git a/src/apm_cli/integration/instruction_integrator.py b/src/apm_cli/integration/instruction_integrator.py index 3fad00cea..7f65fb47d 100644 --- a/src/apm_cli/integration/instruction_integrator.py +++ b/src/apm_cli/integration/instruction_integrator.py @@ -60,6 +60,7 @@ def integrate_instructions_for_target( force: bool = False, managed_files: Optional[Set[str]] = None, diagnostics=None, + user_scope: bool = False, ) -> IntegrationResult: """Integrate instructions for a single *target*. @@ -72,9 +73,9 @@ def integrate_instructions_for_target( if not mapping: return IntegrationResult(0, 0, 0, []) - effective_root = mapping.deploy_root or target.root_dir + effective_root = mapping.deploy_root or target.effective_root(user_scope=user_scope) target_root = project_root / effective_root - if not target.auto_create and not (project_root / target.root_dir).is_dir(): + if not target.auto_create and not (project_root / target.effective_root(user_scope=user_scope)).is_dir(): return IntegrationResult(0, 0, 0, []) self.init_link_resolver(package_info, project_root) @@ -134,12 +135,13 @@ def sync_for_target( apm_package, project_root: Path, managed_files: Optional[Set[str]] = None, + user_scope: bool = False, ) -> Dict[str, int]: """Remove APM-managed instruction files for a single *target*.""" mapping = target.primitives.get("instructions") if not mapping: return {"files_removed": 0, "errors": 0} - effective_root = mapping.deploy_root or target.root_dir + effective_root = mapping.deploy_root or target.effective_root(user_scope=user_scope) prefix = f"{effective_root}/{mapping.subdir}/" legacy_dir = project_root / effective_root / mapping.subdir legacy_pattern = ( diff --git a/src/apm_cli/integration/prompt_integrator.py b/src/apm_cli/integration/prompt_integrator.py index 7b7acad86..aa862258d 100644 --- a/src/apm_cli/integration/prompt_integrator.py +++ b/src/apm_cli/integration/prompt_integrator.py @@ -84,13 +84,14 @@ def integrate_prompts_for_target( force: bool = False, managed_files: Optional[Set[str]] = None, diagnostics=None, + user_scope: bool = False, ) -> IntegrationResult: """Integrate prompts for a single *target*.""" mapping = target.primitives.get("prompts") if not mapping: return IntegrationResult(0, 0, 0, []) - if not target.auto_create and not (project_root / target.root_dir).is_dir(): + if not target.auto_create and not (project_root / target.effective_root(user_scope=user_scope)).is_dir(): return IntegrationResult(0, 0, 0, []) return self.integrate_package_prompts( @@ -105,12 +106,13 @@ def sync_for_target( apm_package, project_root: Path, managed_files: Optional[Set[str]] = None, + user_scope: bool = False, ) -> Dict[str, int]: """Remove APM-managed prompt files for a single *target*.""" mapping = target.primitives.get("prompts") if not mapping: return {"files_removed": 0, "errors": 0} - effective_root = mapping.deploy_root or target.root_dir + effective_root = mapping.deploy_root or target.effective_root(user_scope=user_scope) prefix = f"{effective_root}/{mapping.subdir}/" legacy_dir = project_root / effective_root / mapping.subdir return self.sync_remove_files( From 586a1330efd28073fec7831cda3ccc5dacb72316 Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Thu, 2 Apr 2026 12:58:14 +0200 Subject: [PATCH 3/4] fix: correct is_primary logic and skill_skipped return in skill integrator - 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 <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/integration/skill_integrator.py | 26 +++++++++++-- .../unit/integration/test_skill_integrator.py | 38 ++++++++++--------- 2 files changed, 43 insertions(+), 21 deletions(-) diff --git a/src/apm_cli/integration/skill_integrator.py b/src/apm_cli/integration/skill_integrator.py index bf0df8555..fbad57c2e 100644 --- a/src/apm_cli/integration/skill_integrator.py +++ b/src/apm_cli/integration/skill_integrator.py @@ -611,12 +611,11 @@ def _promote_sub_skills_standalone( owned_by = self._build_skill_ownership_map(project_root) count = 0 all_deployed: list[Path] = [] + primary_assigned = False for idx, target in enumerate(targets): if not target.supports("skills"): continue - - is_primary = (idx == 0) # first active target owns diagnostics skills_mapping = target.primitives["skills"] effective_root = skills_mapping.deploy_root or target.effective_root(user_scope=user_scope) @@ -625,6 +624,10 @@ def _promote_sub_skills_standalone( if not target.auto_create and not target_root_dir.is_dir(): continue + is_primary = not primary_assigned + if is_primary: + primary_assigned = True + target_skills_root = project_root / effective_root / "skills" target_skills_root.mkdir(parents=True, exist_ok=True) @@ -721,12 +724,12 @@ def _integrate_native_skill( owned_by = self._build_skill_ownership_map(project_root) sub_skills_dir = package_path / ".apm" / "skills" primary_effective_root = None + primary_assigned = False for idx, target in enumerate(targets): if not target.supports("skills"): continue - is_primary = (idx == 0) # first active target owns diagnostics skills_mapping = target.primitives["skills"] effective_root = skills_mapping.deploy_root or target.effective_root(user_scope=user_scope) @@ -738,6 +741,10 @@ def _integrate_native_skill( if primary_effective_root is None: primary_effective_root = effective_root + is_primary = not primary_assigned + if is_primary: + primary_assigned = True + target_skill_dir = project_root / effective_root / "skills" / skill_name if is_primary: @@ -770,6 +777,19 @@ def _integrate_native_skill( ) all_target_paths.extend(sub_deployed) + if not all_target_paths: + # Every target was skipped (auto_create guard) + return SkillIntegrationResult( + skill_created=False, + skill_updated=False, + skill_skipped=True, + skill_path=None, + references_copied=0, + links_resolved=0, + sub_skills_promoted=0, + target_paths=[], + ) + # Count unique sub-skills from primary target only primary_root = project_root / (primary_effective_root or ".github") / "skills" sub_skills_count = sum( diff --git a/tests/unit/integration/test_skill_integrator.py b/tests/unit/integration/test_skill_integrator.py index 52b03f0b7..2488d0a78 100644 --- a/tests/unit/integration/test_skill_integrator.py +++ b/tests/unit/integration/test_skill_integrator.py @@ -394,7 +394,7 @@ def test_integrate_package_skill_processes_virtual_subdirectory_packages(self): assert virtual_dep_ref.is_virtual # Sanity check assert virtual_dep_ref.is_virtual_subdirectory() # This is a subdirectory, not file - # Has SKILL.md → CLAUDE_SKILL type + # Has SKILL.md -> CLAUDE_SKILL type package_info = self._create_package_info( install_path=package_dir, name="mcp-builder", @@ -506,7 +506,7 @@ def test_sync_integration_returns_zero_stats(self): def test_sync_integration_removes_orphaned_subdirectory_skill(self): """Test that sync removes skills for uninstalled subdirectory packages. - This tests the full install → uninstall flow for virtual subdirectory packages + This tests the full install -> uninstall flow for virtual subdirectory packages like ComposioHQ/awesome-claude-skills/mcp-builder. """ # Simulate an installed skill from a subdirectory package @@ -1882,7 +1882,7 @@ def test_sync_keeps_installed_skills_in_both_locations(self): (claude_skill / "SKILL.md").write_text("# Installed Skill\n") # Mock apm_package with this dependency installed - # "owner/installed-skill" → skill dir name "installed-skill" + # "owner/installed-skill" -> skill dir name "installed-skill" dep_ref = DependencyReference.parse("owner/installed-skill") apm_package = Mock() apm_package.get_apm_dependencies.return_value = [dep_ref] @@ -2481,7 +2481,7 @@ def test_content_identical_sub_skill_skipped(self): target = self.project_root / ".github" / "skills" / "my-skill" / "SKILL.md" assert target.exists() - # Second install — content identical; copytree/rmtree should NOT be called + # Second install -- content identical; copytree/rmtree should NOT be called from unittest.mock import patch with patch("shutil.rmtree") as mock_rm, patch("shutil.copytree") as mock_cp: self.integrator.integrate_package_skill(pkg_info, self.project_root) @@ -2503,7 +2503,7 @@ def test_content_different_sub_skill_replaced(self): target = self.project_root / ".github" / "skills" / "my-skill" / "SKILL.md" target.write_text("# Modified by user") - # Second install — content differs + # Second install -- content differs self.integrator.integrate_package_skill(pkg_info, self.project_root) # Should be overwritten with original content @@ -2522,7 +2522,7 @@ def test_user_authored_skill_skipped_without_force(self): package_dir = self._create_package_with_sub_skills("pkg", sub_skills=["my-skill"]) pkg_info = self._create_package_info(name="pkg", install_path=package_dir) - # managed_files is set but does NOT contain this skill → user-authored + # managed_files is set but does NOT contain this skill -> user-authored managed_files = set() from apm_cli.utils.diagnostics import DiagnosticCollector @@ -2575,7 +2575,7 @@ def test_cross_package_overwrite_records_diagnostic(self): package_dir = self._create_package_with_sub_skills("my-pkg", sub_skills=["shared-skill"]) pkg_info = self._create_package_info(name="my-pkg", install_path=package_dir) - # Managed files includes this skill → it's APM-managed + # Managed files includes this skill -> it's APM-managed managed_files = {".github/skills/shared-skill"} from apm_cli.utils.diagnostics import DiagnosticCollector, CATEGORY_OVERWRITE @@ -2619,7 +2619,7 @@ def test_self_overwrite_silent_no_diagnostic(self): diagnostics=diag, managed_files=managed_files, force=False, ) - # Self-overwrite — no diagnostics should be recorded + # Self-overwrite -- no diagnostics should be recorded assert not diag.has_diagnostics # Content should be updated @@ -2702,7 +2702,7 @@ def _create_package_with_sub_skills(self, name="parent-skill", sub_skills=None): ) return package_dir - # ========== Test: Opt-in guard — no .cursor/ means no deployment ========== + # ========== Test: Opt-in guard -- no .cursor/ means no deployment ========== def test_no_cursor_deployment_when_cursor_dir_missing(self): """Skills should NOT deploy to .cursor/skills/ when .cursor/ doesn't exist.""" @@ -3144,7 +3144,7 @@ def _create_package_info( package_type=package_type, ) - # ========== integrate_package_skill — user_scope path selection ========== + # ========== integrate_package_skill -- user_scope path selection ========== def test_user_scope_deploys_to_user_root_dir(self): """With user_scope=True, skills deploy to user_root_dir (.config/opencode).""" @@ -3190,7 +3190,7 @@ def test_project_scope_deploys_to_root_dir(self): expected = self.project_root / ".opencode" / "skills" / "my-skill" / "SKILL.md" assert expected.exists() - # ========== auto_create guard — no deployment when dir missing ========== + # ========== auto_create guard -- no deployment when dir missing ========== def test_auto_create_false_skips_when_dir_missing(self): """auto_create=False targets are skipped when their root dir doesn't exist.""" @@ -3209,7 +3209,8 @@ def test_auto_create_false_skips_when_dir_missing(self): pkg, self.project_root, targets=[opencode], user_scope=False, ) - # Skipped entirely — nothing created + # Skipped entirely -- nothing created + assert result.skill_skipped is True assert len(result.target_paths) == 0 assert not (self.project_root / ".opencode" / "skills").exists() @@ -3230,6 +3231,7 @@ def test_auto_create_false_skips_user_scope_when_user_dir_missing(self): pkg, self.project_root, targets=[opencode], user_scope=True, ) + assert result.skill_skipped is True assert len(result.target_paths) == 0 assert not (self.project_root / ".config" / "opencode" / "skills").exists() @@ -3253,7 +3255,7 @@ def test_auto_create_true_creates_dir(self): assert result.skill_created is True assert (self.project_root / ".github" / "skills" / "my-skill" / "SKILL.md").exists() - # ========== _promote_sub_skills_standalone — user_scope ========== + # ========== _promote_sub_skills_standalone -- user_scope ========== def test_sub_skill_promotion_uses_user_root(self): """Sub-skills promoted at user_scope use user_root_dir.""" @@ -3304,7 +3306,7 @@ def test_sub_skill_standalone_skips_auto_create_false(self): assert len(result.target_paths) == 0 assert not (self.project_root / ".opencode" / "skills").exists() - # ========== copy_skill_to_target — user_scope ========== + # ========== copy_skill_to_target -- user_scope ========== def test_copy_skill_to_target_user_scope(self): """copy_skill_to_target with user_scope=True uses user_root_dir.""" @@ -3358,7 +3360,7 @@ def test_copy_skill_to_target_auto_create_guard(self): assert len(deployed) == 0 assert not (self.project_root / ".opencode" / "skills").exists() - # ========== primary_root — dynamic, not hardcoded .github ========== + # ========== primary_root -- dynamic, not hardcoded .github ========== def test_primary_root_uses_first_active_target(self): """Sub-skill counting uses effective root from first active target, not hardcoded .github.""" @@ -3379,10 +3381,10 @@ def test_primary_root_uses_first_active_target(self): pkg, self.project_root, targets=[opencode], user_scope=True, ) - # Sub-skill should be counted — it's under the primary target's root + # Sub-skill should be counted -- it's under the primary target's root assert result.sub_skills_promoted == 1 - # ========== backward compatibility — user_scope default ========== + # ========== backward compatibility -- user_scope default ========== def test_user_scope_defaults_to_false(self): """Calling without user_scope uses project-scope (root_dir).""" @@ -3395,7 +3397,7 @@ def test_user_scope_defaults_to_false(self): (skill_source / "SKILL.md").write_text("---\nname: my-skill\n---\n# My Skill") pkg = self._create_package_info(name="my-skill", install_path=skill_source) - # Call WITHOUT user_scope — should work exactly as before + # Call WITHOUT user_scope -- should work exactly as before result = self.integrator.integrate_package_skill( pkg, self.project_root, targets=[copilot], ) From 214c9bff05a813c45669fdd6987bee81c9ea18a0 Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Thu, 2 Apr 2026 13:39:35 +0200 Subject: [PATCH 4/4] docs: add CHANGELOG entry for user-scope deploy path fix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91ac1fb41..9bc0160ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- `apm install -g` now deploys files to the correct user-scope directories for all integrators; fixes OpenCode deploying to `.opencode/` instead of `.config/opencode/` and similar path mismatches for Copilot (#537, #530) + ## [0.8.9] - 2026-03-31 ### Fixed