From 0ef559cca54675ae83eec8ea0bcffe1e693c84b4 Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Thu, 2 Apr 2026 12:01:54 +0200 Subject: [PATCH 1/2] fix: skip unsupported Copilot CLI primitives at user scope When running apm install --global, APM incorrectly deployed instructions to ~/.copilot/instructions/. Per the official Copilot CLI config directory reference, ~/.copilot/ only supports agents/, skills/, and hooks/. - Add instructions to Copilot unsupported_user_primitives - Fix user-scope root_dir resolution for integrators - Update tests and documentation Ref: https://docs.github.com/en/copilot/reference/copilot-cli-reference/cli-config-dir-reference Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/src/content/docs/guides/dependencies.md | 2 +- src/apm_cli/commands/install.py | 14 +++++++++++++- src/apm_cli/integration/targets.py | 6 +++--- tests/unit/core/test_scope.py | 8 +++++--- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/docs/src/content/docs/guides/dependencies.md b/docs/src/content/docs/guides/dependencies.md index f8ba98006..b0c0778c9 100644 --- a/docs/src/content/docs/guides/dependencies.md +++ b/docs/src/content/docs/guides/dependencies.md @@ -310,7 +310,7 @@ Coverage varies by target and primitive type: | Target | Status | User-level dir | Primitives | Not supported | |--------|--------|---------------|------------|---------------| | Claude Code | Supported | `~/.claude/` | Skills, agents, commands, hooks, instructions | -- | -| Copilot CLI | Partial | `~/.copilot/` | Skills, agents, instructions, hooks | Prompts (not supported by Copilot CLI) | +| Copilot CLI | Partial | `~/.copilot/` | Skills, agents, hooks | Prompts, instructions | | Cursor | Partial | `~/.cursor/` | Skills, agents, hooks | Rules | | OpenCode | Partial | `~/.config/opencode/` | Skills, agents, commands | Hooks | diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 06b562aca..72085d3a1 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -1009,6 +1009,18 @@ def _log_integration(msg): "instructions": (instruction_integrator, "integrate_instructions_for_target", "instructions"), } + # At user scope, override root_dir with the user-scope directory so + # integrators deploy to e.g. ~/.copilot/ instead of ~/.github/. + if _user_scope: + from dataclasses import replace as _dc_replace + _adjusted_targets = [] + for _t in targets: + if _t.user_root_dir and _t.user_root_dir != _t.root_dir: + _adjusted_targets.append(_dc_replace(_t, root_dir=_t.user_root_dir)) + else: + _adjusted_targets.append(_t) + targets = _adjusted_targets + # --- target x primitive dispatch loop --- for _target in targets: # Skip entire target when user scope is not supported @@ -1331,7 +1343,7 @@ def download_callback(dep_ref, modules_dir, parent_chain=""): ) try: - dependency_graph = resolver.resolve_dependencies(project_root) + dependency_graph = resolver.resolve_dependencies(apm_dir) # Verbose: show resolved tree summary if logger: diff --git a/src/apm_cli/integration/targets.py b/src/apm_cli/integration/targets.py index 4f54e0d5f..438e23f18 100644 --- a/src/apm_cli/integration/targets.py +++ b/src/apm_cli/integration/targets.py @@ -121,8 +121,8 @@ def supports_at_user_scope(self, primitive: str) -> bool: KNOWN_TARGETS: Dict[str, TargetProfile] = { # Copilot (GitHub) -- at user scope, Copilot CLI reads ~/.copilot/ - # instead of ~/.github/. Prompts are not supported at user scope. - # Ref: https://docs.github.com/en/copilot/how-tos/copilot-cli/customize-copilot/create-custom-agents-for-cli + # instead of ~/.github/. Prompts and instructions are not supported at user scope. + # Ref: https://docs.github.com/en/copilot/reference/copilot-cli-reference/cli-config-dir-reference "copilot": TargetProfile( name="copilot", root_dir=".github", @@ -147,7 +147,7 @@ def supports_at_user_scope(self, primitive: str) -> bool: detect_by_dir=True, user_supported="partial", user_root_dir=".copilot", - unsupported_user_primitives=("prompts",), + unsupported_user_primitives=("prompts", "instructions"), ), # Claude Code -- ~/.claude/ is the documented user-level config directory. # All primitives are supported at user scope. diff --git a/tests/unit/core/test_scope.py b/tests/unit/core/test_scope.py index 3c978b5c3..811457238 100644 --- a/tests/unit/core/test_scope.py +++ b/tests/unit/core/test_scope.py @@ -192,6 +192,7 @@ def test_claude_uses_default_root_at_user_scope(self): def test_copilot_unsupported_user_primitives(self): assert "prompts" in KNOWN_TARGETS["copilot"].unsupported_user_primitives + assert "instructions" in KNOWN_TARGETS["copilot"].unsupported_user_primitives def test_effective_root_project_scope(self): assert KNOWN_TARGETS["copilot"].effective_root(user_scope=False) == ".github" @@ -207,9 +208,10 @@ def test_supports_at_user_scope_true(self): assert KNOWN_TARGETS["claude"].supports_at_user_scope("commands") is True def test_supports_at_user_scope_partial(self): - # Copilot supports agents at user scope but not prompts + # Copilot supports agents at user scope but not prompts or instructions assert KNOWN_TARGETS["copilot"].supports_at_user_scope("agents") is True assert KNOWN_TARGETS["copilot"].supports_at_user_scope("prompts") is False + assert KNOWN_TARGETS["copilot"].supports_at_user_scope("instructions") is False def test_supports_at_user_scope_cursor_partial(self): # Cursor supports agents at user scope but not instructions @@ -264,8 +266,8 @@ def test_warn_message_includes_partial_targets(self): def test_warn_message_includes_unsupported_primitives(self): msg = warn_unsupported_user_scope() - # Copilot excludes prompts - assert "copilot (prompts)" in msg + # Copilot excludes prompts and instructions + assert "copilot (prompts, instructions)" in msg # Cursor excludes instructions assert "cursor (instructions)" in msg # OpenCode excludes hooks From 748600355210e92e5c8178fbc3bd7b1f0d91e246 Mon Sep 17 00:00:00 2001 From: Sergio Sisternes Date: Thu, 2 Apr 2026 12:18:14 +0200 Subject: [PATCH 2/2] fix: thread effective_root(user_scope) across integration stack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Copilot review feedback: make user_root_dir a first-class deploy root in validate_deploy_path, partition_managed_files, and the uninstall engine — not just in _integrate_package_primitives. - get_integration_prefixes(user_scope=True) includes user-scope prefixes - validate_deploy_path accepts user-scope paths when user_scope=True - partition_managed_files routes user-scope paths correctly - Uninstall engine uses effective_root(user_scope) for path resolution - Add 8 tests for user-scope integration Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/apm_cli/commands/install.py | 2 +- src/apm_cli/commands/uninstall/cli.py | 5 +- src/apm_cli/commands/uninstall/engine.py | 8 +- src/apm_cli/integration/base_integrator.py | 10 ++- src/apm_cli/integration/targets.py | 13 ++- .../integration/test_data_driven_dispatch.py | 90 +++++++++++++++++++ 6 files changed, 117 insertions(+), 11 deletions(-) diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 72085d3a1..8462024c7 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -2371,7 +2371,7 @@ def _collect_descendants(node, visited=None): # validate_deploy_path() is the safety gate: it rejects path-traversal, # requires .github/ or .claude/ prefix, and checks the resolved path # stays within project_root — so rmtree is safe here. - if BaseIntegrator.validate_deploy_path(_orphan_path, project_root): + if BaseIntegrator.validate_deploy_path(_orphan_path, project_root, user_scope=_is_user): _target = project_root / _orphan_path if _target.exists(): try: diff --git a/src/apm_cli/commands/uninstall/cli.py b/src/apm_cli/commands/uninstall/cli.py index b49b6ecc0..8bd453c44 100644 --- a/src/apm_cli/commands/uninstall/cli.py +++ b/src/apm_cli/commands/uninstall/cli.py @@ -179,7 +179,10 @@ def uninstall(ctx, packages, dry_run, verbose, global_): cleaned = {"prompts": 0, "agents": 0, "skills": 0, "commands": 0, "hooks": 0, "instructions": 0} try: apm_package = APMPackage.from_apm_yml(manifest_path) - cleaned = _sync_integrations_after_uninstall(apm_package, deploy_root, all_deployed_files, logger) + cleaned = _sync_integrations_after_uninstall( + apm_package, deploy_root, all_deployed_files, logger, + user_scope=scope is InstallScope.USER, + ) except Exception: pass # Best effort cleanup diff --git a/src/apm_cli/commands/uninstall/engine.py b/src/apm_cli/commands/uninstall/engine.py index 27a37ea8e..b9cf807bb 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=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 @@ -238,7 +238,7 @@ def _sync_integrations_after_uninstall(apm_package, project_root, all_deployed_f sync_managed = all_deployed_files if all_deployed_files else None if sync_managed is not None: - _buckets = BaseIntegrator.partition_managed_files(sync_managed) + _buckets = BaseIntegrator.partition_managed_files(sync_managed, user_scope=user_scope) else: _buckets = None @@ -266,7 +266,7 @@ def _sync_integrations_after_uninstall(apm_package, project_root, all_deployed_f if not _entry: continue _integrator, _counter_key = _entry - _effective_root = _mapping.deploy_root or _target.root_dir + _effective_root = _mapping.deploy_root or _target.effective_root(user_scope=user_scope) _deploy_dir = project_root / _effective_root / _mapping.subdir if not _deploy_dir.exists(): continue @@ -288,7 +288,7 @@ def _sync_integrations_after_uninstall(apm_package, project_root, all_deployed_f for t in KNOWN_TARGETS.values(): if t.supports("skills"): sm = t.primitives["skills"] - er = sm.deploy_root or t.root_dir + er = sm.deploy_root or t.effective_root(user_scope=user_scope) if (project_root / er / "skills").exists(): _skill_dirs_exist = True break diff --git a/src/apm_cli/integration/base_integrator.py b/src/apm_cli/integration/base_integrator.py index 8df8dc5c0..8c04344f5 100644 --- a/src/apm_cli/integration/base_integrator.py +++ b/src/apm_cli/integration/base_integrator.py @@ -96,15 +96,16 @@ def normalize_managed_files(managed_files: Optional[Set[str]]) -> Optional[Set[s # Known integration prefixes that APM is allowed to deploy/remove under. # Derived from ``targets.KNOWN_TARGETS`` so adding a target auto-propagates. @staticmethod - def _get_integration_prefixes() -> tuple: + def _get_integration_prefixes(user_scope: bool = False) -> tuple: from apm_cli.integration.targets import get_integration_prefixes - return get_integration_prefixes() + return get_integration_prefixes(user_scope=user_scope) @staticmethod def validate_deploy_path( rel_path: str, project_root: Path, allowed_prefixes: tuple | None = None, + user_scope: bool = False, ) -> bool: """Return True if *rel_path* is safe for APM to deploy or remove. @@ -117,7 +118,7 @@ def validate_deploy_path( 3. Resolves within *project_root* """ if allowed_prefixes is None: - allowed_prefixes = BaseIntegrator._get_integration_prefixes() + allowed_prefixes = BaseIntegrator._get_integration_prefixes(user_scope=user_scope) if ".." in rel_path: return False if not rel_path.startswith(allowed_prefixes): @@ -156,6 +157,7 @@ def partition_bucket_key(prim_name: str, target_name: str) -> str: @staticmethod def partition_managed_files( managed_files: Set[str], + user_scope: bool = False, ) -> dict: """Partition *managed_files* by integration prefix in a single pass. @@ -184,7 +186,7 @@ def partition_managed_files( for target in KNOWN_TARGETS.values(): for prim_name, mapping in target.primitives.items(): - 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}/" if mapping.subdir else f"{effective_root}/" if prim_name == "skills": skill_prefixes.append(prefix) diff --git a/src/apm_cli/integration/targets.py b/src/apm_cli/integration/targets.py index 438e23f18..b595dbfe9 100644 --- a/src/apm_cli/integration/targets.py +++ b/src/apm_cli/integration/targets.py @@ -246,7 +246,7 @@ def supports_at_user_scope(self, primitive: str) -> bool: } -def get_integration_prefixes() -> tuple: +def get_integration_prefixes(user_scope: bool = False) -> tuple: """Return all known target root prefixes as a tuple. Used by ``BaseIntegrator.validate_deploy_path`` so the allow-list @@ -254,13 +254,24 @@ def get_integration_prefixes() -> tuple: Includes prefixes from ``deploy_root`` overrides (e.g. ``.agents/`` for Codex skills) so cross-root paths pass security validation. + + When *user_scope* is ``True``, user-scope prefixes (from + ``user_root_dir``) are included alongside project-scope prefixes + so that validation works for both scopes. """ prefixes: list[str] = [] seen: set[str] = set() for t in KNOWN_TARGETS.values(): + # Always include project-scope prefix if t.prefix not in seen: seen.add(t.prefix) prefixes.append(t.prefix) + # At user scope, also include user_root_dir prefix + if user_scope and t.user_root_dir: + user_prefix = f"{t.user_root_dir}/" + if user_prefix not in seen: + seen.add(user_prefix) + prefixes.append(user_prefix) for m in t.primitives.values(): if m.deploy_root is not None: dp = f"{m.deploy_root}/" diff --git a/tests/unit/integration/test_data_driven_dispatch.py b/tests/unit/integration/test_data_driven_dispatch.py index df8bc935e..696fc093a 100644 --- a/tests/unit/integration/test_data_driven_dispatch.py +++ b/tests/unit/integration/test_data_driven_dispatch.py @@ -520,3 +520,93 @@ def test_deploy_root_validation(self): root = Path("/fake/project") assert BaseIntegrator.validate_deploy_path(".agents/skills/my-skill/SKILL.md", root) assert BaseIntegrator.validate_deploy_path(".codex/agents/my-agent.toml", root) + + +# =================================================================== +# 8. TestUserScopeIntegration +# =================================================================== + +class TestUserScopeIntegration: + """Verify user_scope parameter threads through prefixes, validation, + and partition routing.""" + + def test_get_integration_prefixes_default_no_user_dirs(self): + """Without user_scope, user_root_dir prefixes are absent.""" + from apm_cli.integration.targets import get_integration_prefixes, KNOWN_TARGETS + prefixes = get_integration_prefixes() + for t in KNOWN_TARGETS.values(): + if t.user_root_dir and t.user_root_dir != t.root_dir: + assert f"{t.user_root_dir}/" not in prefixes + + def test_get_integration_prefixes_user_scope_includes_user_dirs(self): + """user_scope=True adds user_root_dir prefixes (.copilot/, etc.).""" + from apm_cli.integration.targets import get_integration_prefixes, KNOWN_TARGETS + prefixes = get_integration_prefixes(user_scope=True) + for t in KNOWN_TARGETS.values(): + if t.user_root_dir: + assert f"{t.user_root_dir}/" in prefixes + # Project-scope prefixes still present + assert t.prefix in prefixes + + def test_get_integration_prefixes_user_scope_no_duplicates(self): + """No duplicate prefixes even when user_root_dir equals root_dir.""" + from apm_cli.integration.targets import get_integration_prefixes + prefixes = get_integration_prefixes(user_scope=True) + assert len(prefixes) == len(set(prefixes)) + + def test_validate_deploy_path_user_scope_copilot(self): + """validate_deploy_path accepts .copilot/ paths at user scope.""" + root = Path("/fake/home") + assert BaseIntegrator.validate_deploy_path( + ".copilot/agents/my-agent.md", root, user_scope=True, + ) + # Without user_scope, .copilot/ should be rejected + assert not BaseIntegrator.validate_deploy_path( + ".copilot/agents/my-agent.md", root, user_scope=False, + ) + + def test_validate_deploy_path_user_scope_cursor(self): + """validate_deploy_path accepts .cursor/ paths at user scope.""" + root = Path("/fake/home") + # .cursor/ is already the root_dir for cursor target, so it should + # be accepted even without user_scope + from apm_cli.integration.targets import KNOWN_TARGETS + cursor_target = KNOWN_TARGETS.get("cursor") + if cursor_target and cursor_target.user_root_dir == ".cursor": + # .cursor/ is already the project root_dir, so accepted either way + assert BaseIntegrator.validate_deploy_path( + ".cursor/rules/my-rule.md", root, user_scope=False, + ) + + def test_validate_deploy_path_backward_compat(self): + """Default user_scope=False preserves existing behaviour.""" + root = Path("/fake/project") + assert BaseIntegrator.validate_deploy_path(".github/prompts/test.md", root) + assert BaseIntegrator.validate_deploy_path(".claude/commands/test.md", root) + assert not BaseIntegrator.validate_deploy_path("../escape.md", root) + + def test_partition_managed_files_user_scope_routes_copilot(self): + """partition_managed_files routes .copilot/ paths at user scope.""" + managed = { + ".copilot/agents/my-agent.md", + ".copilot/prompts/test.prompt.md", + } + buckets = BaseIntegrator.partition_managed_files(managed, user_scope=True) + # At user scope, copilot's effective_root is .copilot, so + # .copilot/agents/ maps to agents_github (copilot alias) and + # .copilot/prompts/ maps to prompts (copilot alias) + from apm_cli.integration.targets import KNOWN_TARGETS + copilot = KNOWN_TARGETS["copilot"] + if copilot.user_root_dir == ".copilot": + assert ".copilot/agents/my-agent.md" in buckets.get("agents_github", set()) + assert ".copilot/prompts/test.prompt.md" in buckets.get("prompts", set()) + + def test_partition_managed_files_default_backward_compat(self): + """Default user_scope=False produces same buckets as before.""" + managed = { + ".github/prompts/test.prompt.md", + ".claude/commands/test.md", + } + buckets = BaseIntegrator.partition_managed_files(managed) + assert ".github/prompts/test.prompt.md" in buckets.get("prompts", set()) + assert ".claude/commands/test.md" in buckets.get("commands", set())