fix: scope-resolved target profiles + Claude Code instructions

[danielmeppiel]

Description

Introduces TargetProfile.for_scope() and resolve_targets() so scope resolution happens exactly once -- no parameter threading, no caller responsibility. Supersedes #536 and #537. Builds on #527 (already merged).

Architecture: scope-resolved targets

The root cause of user-scope bugs: every code path that touches target.root_dir must choose the right value for the scope. Both #536 (dataclasses.replace at dispatch) and #537 (thread user_scope through every method) fix the install path but both miss uninstall Phase 2 re-integration, and neither handles multi-level user_root_dir values like .config/opencode in partition routing.

Solution: resolve scope on the data object itself, once, at the entry point.

• 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 + filtering
• Integrators read target.root_dir directly -- always correct for the current scope
• partition_managed_files(targets=) uses trie-based O(depth) routing, fixing .config/opencode/ multi-level root paths
• validate_deploy_path(targets=) derives prefixes from resolved targets
• Uninstall Phase 2 re-integration uses the same resolved targets -- eliminates wrong-path deployment bug

Skill integrator scope fixes (#538, #539)

• sync_integration() now derives skill prefixes dynamically from targets instead of hardcoded strings -- handles .copilot/skills/, .config/opencode/skills/, etc.
• Uninstall Phase 2 passes resolved targets to integrate_package_skill()
• copy_skill_to_target() adds auto_create guard for targets without existing directories

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, #538, #539
Supersedes #536, #537

Type of change

• Bug fix
• New feature
• Documentation
• Maintenance / refactor

Testing

• All existing tests pass (3,506 unit tests)
• 40+ new tests covering scope resolution, partition routing, skill sync, and backward compatibility

[Copilot]

Pull request overview

This PR centralizes scope resolution for integration targets (project vs user scope) and expands Claude Code support by deploying .instructions.md as .claude/rules/*.md with applyTo: converted to Claude's paths: frontmatter, while extending hook path rewriting to cover additional platform-specific keys.

Changes:

• Add TargetProfile.for_scope() and resolve_targets() to resolve scope exactly once and remove scope-threading from downstream integration logic.
• Add Claude Code instructions support (claude_rules) with conversion + sync behavior that avoids glob-deleting user-authored .md files.
• Extend hook script-path rewriting to include additional VS Code OS-specific keys (windows, linux, osx) and add tests for these cases.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/apm_cli/integration/targets.py	Adds scope-resolved target profiles (for_scope) and resolve_targets, plus Claude instructions mapping and Copilot user-scope primitive filtering.
src/apm_cli/integration/base_integrator.py	Adds targets= plumbing for validation/partitioning and changes partition routing to longest-prefix matching.
src/apm_cli/commands/install.py	Switches target detection to resolve_targets, adjusts logging/dir creation, uses targets= for deploy-path validation, and fixes dependency resolution root for user scope.
src/apm_cli/commands/uninstall/engine.py	Uses scope-resolved targets for both Phase 1 cleanup and Phase 2 re-integration, and partitions managed files using resolved targets.
src/apm_cli/commands/uninstall/cli.py	Passes user_scope into uninstall integration sync.
src/apm_cli/integration/instruction_integrator.py	Adds Claude rules conversion/copy/sync + format-driven instruction dispatch.
src/apm_cli/integration/hook_integrator.py	Expands script-path key handling via HOOK_COMMAND_KEYS and updates rewrite loops accordingly.
tests/unit/integration/test_instruction_integrator.py	Adds conversion + integration + sync tests for Claude rules.
tests/unit/integration/test_hook_integrator.py	Adds rewrite tests for windows/linux/osx keys and expands coverage of multi-key scenarios.
tests/unit/integration/test_data_driven_dispatch.py	Adds bucket alias/routing tests for Claude rules and validates prefix/partition behavior with explicit targets.
tests/unit/core/test_scope.py	Updates Copilot user-scope expectations to include instructions as unsupported.
docs/src/content/docs/integrations/ide-tool-integration.md	Documents .claude/rules/*.md as a managed Claude integration location and updates sync semantics to include rules.
docs/src/content/docs/guides/dependencies.md	Updates Copilot CLI user-scope primitive support table (removes instructions).
CHANGELOG.md	Adds Unreleased entries for Claude rules + scope-resolved targets + user-scope fixes.
uv.lock	Updates editable apm-cli version entry.

Comments suppressed due to low confidence (1)

src/apm_cli/integration/base_integrator.py:133

• validate_deploy_path() still uses a naive substring check (".." in rel_path) for path traversal. This can be bypassed with mixed separators or path normalization edge cases, and it duplicates logic that should live in utils/path_security.py. Consider switching to validate_path_segments(rel_path, context=...) (after normalizing backslashes) and ensure_path_within(resolved_target, project_root) as the authoritative traversal/containment checks.

        if allowed_prefixes is None:
            allowed_prefixes = BaseIntegrator._get_integration_prefixes(targets=targets)
        if ".." in rel_path:
            return False
        if not rel_path.startswith(allowed_prefixes):
            return False
        target = project_root / rel_path
        try:
            if not target.resolve().is_relative_to(project_root.resolve()):
                return False

[danielmeppiel]

What this PR does

User-scope deployment (apm install -g) was broken across multiple code paths because every caller had to remember to use target.effective_root(user_scope=True) instead of target.root_dir. Many callers forgot -- causing skills to deploy to ~/.github/ instead of ~/.copilot/, uninstall to miss cleanup, and partition routing to fail for multi-level paths like ~/.config/opencode/.

This PR eliminates the problem at its root: scope resolution happens once, on the data object itself, before any code sees it.

• TargetProfile.for_scope(user_scope) returns a frozen copy with root_dir already set to the correct value and unsupported primitives filtered out
• resolve_targets() is the single entry point -- callers get profiles where root_dir is always correct
• Every downstream consumer (integrators, partition routing, validation, install, uninstall) just reads target.root_dir -- no scope awareness needed

How it solves each linked issue

#530 -- skills deploy to wrong user-scope path: The core bug. resolve_targets(project_root, user_scope=True) now returns Copilot with root_dir=".copilot" and OpenCode with root_dir=".config/opencode". All integrators read root_dir directly, so skills (and agents, commands, instructions) land in the right place. Install and uninstall both use the same resolved targets, including Phase 2 re-integration which was previously broken.

#538 -- copy_skill_to_target() ignores user_scope: Fixed by passing resolved targets from the uninstall engine to integrate_package_skill(). Added auto_create guard to copy_skill_to_target() so targets without existing directories are skipped instead of silently created.

#539 -- sync_integration() uses hardcoded skill prefixes: Refactored to derive prefixes dynamically from the targets list. The old code had five hardcoded blocks (.github/skills/, .claude/skills/, .cursor/skills/, .opencode/skills/, .agents/skills/) that couldn't match user-scope paths like .copilot/skills/. Now it loops over whatever targets are passed, building prefixes from deploy_root or target.root_dir.

Also cherry-picks from superseded PRs: Copilot instructions added to unsupported_user_primitives (#536), resolve_dependencies(apm_dir) fix (#536).

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (4)

src/apm_cli/commands/uninstall/engine.py:319

• HookIntegrator.sync_integration() does its own manifest-based removal with hard-coded prefixes (e.g. .github/hooks/, .claude/hooks/). In user scope, resolved targets will deploy hooks under directories like .copilot/hooks/, but this call does not pass any target context and HookIntegrator currently won't match those paths, so user-scope hooks can be left behind on uninstall. Consider updating HookIntegrator's removal logic to derive hook prefixes from the resolved targets (similar to skills) or accept a targets= parameter.

    # Hooks (multi-target, sync_integration handles all targets)
    hook_integrator_cleanup = HookIntegrator()
    result = hook_integrator_cleanup.sync_integration(apm_package, project_root,
                                                      managed_files=_buckets["hooks"] if _buckets else None)
    counts["hooks"] = result.get("files_removed", 0)

src/apm_cli/integration/base_integrator.py:129

• validate_deploy_path() now supports a targets= parameter, but the main removal helper sync_remove_files() (used by prompt/agent/command/instruction integrators) still calls validate_deploy_path(rel_path, project_root) without passing scope-resolved targets/allowed prefixes. As a result, any user-scope deployed path under .copilot/ or .config/opencode/ will fail validation and not be removed during sync/uninstall. Consider extending sync_remove_files() to accept targets (or allowed_prefixes) and wiring it through all sync_for_target() call sites.

    @staticmethod
    def validate_deploy_path(
        rel_path: str,
        project_root: Path,
        allowed_prefixes: tuple | None = None,
        targets=None,
    ) -> bool:
        """Return True if *rel_path* is safe for APM to deploy or remove.

        Centralised security gate for all paths read from ``deployed_files``
        before any filesystem operation.

        When *targets* is provided, allowed prefixes are derived from
        those (scope-resolved) profiles.  Otherwise uses all known
        target prefixes.

        Checks:
        1. No path-traversal components (``..``)
        2. Starts with an allowed integration prefix
        3. Resolves within *project_root*
        """
        if allowed_prefixes is None:
            allowed_prefixes = BaseIntegrator._get_integration_prefixes(targets=targets)
        if ".." in rel_path:
            return False
        if not rel_path.startswith(allowed_prefixes):
            return False

src/apm_cli/integration/skill_integrator.py:905

• The containment check here uses string-prefix matching on str(target.resolve()), which is not a safe substitute for a real path containment test (it can be fooled by symlinks or prefixes like /tmp/root2 starting with /tmp/root). Since this code can rmtree() directories from lockfile-provided paths, it should reuse the centralized guard (e.g., BaseIntegrator.validate_deploy_path(..., targets=...) or ensure_path_within() from utils/path_security.py) rather than ".." in rel_path + startswith() on strings.

                if ".." in rel_path:
                    continue
                target = project_root / rel_path
                if not str(target.resolve()).startswith(str(project_root_resolved)):
                    continue

src/apm_cli/commands/uninstall/engine.py:296

• Phase 1 calls per-target sync_for_target(), but those sync methods ultimately rely on BaseIntegrator.sync_remove_files(), which calls validate_deploy_path() without passing the scope-resolved targets. That means user-scope prefixes like .copilot/ / .config/opencode/ will be rejected by the safety gate and silently not removed even when _target.root_dir is resolved. Suggest threading resolved targets (or allowed_prefixes) into the sync/remove helpers so user-scope uninstall actually deletes tracked files.

    for _target in _resolved_targets:
        for _prim_name, _mapping in _target.primitives.items():
            if _prim_name in ("skills", "hooks"):
                continue
            _entry = _SYNC_DISPATCH.get(_prim_name)
            if not _entry:
                continue
            _integrator, _counter_key = _entry
            _effective_root = _mapping.deploy_root or _target.root_dir
            _deploy_dir = project_root / _effective_root / _mapping.subdir
            if not _deploy_dir.exists():
                continue
            _managed_subset = None
            if _buckets is not None:
                _bucket_key = BaseIntegrator.partition_bucket_key(
                    _prim_name, _target.name
                )
                _managed_subset = _buckets.get(_bucket_key, set())
            result = _integrator.sync_for_target(
                _target, apm_package, project_root,
                managed_files=_managed_subset,
            )
            counts[_counter_key] += result.get("files_removed", 0)

[Copilot]

The comment says we partition against both resolved and unresolved targets, but this call uses the default KNOWN_TARGETS only. At user scope that will not bucket paths under resolved roots like .copilot/... or .config/opencode/..., so _managed_subset (and the skills bucket) can be empty and Phase 1 cleanup will skip removing the actual deployed files. Consider partitioning with a combined target list (resolved + KNOWN_TARGETS) or partition twice (resolved + default) and union the buckets.

This issue also appears on line 274 of the same file.

Suggested change

	# cleaned up during uninstall.
	_buckets = BaseIntegrator.partition_managed_files(sync_managed)
	# cleaned up during uninstall.
	# First partition using the default unresolved/legacy targets, then
	# partition again with resolved targets and merge the buckets.
	_buckets = BaseIntegrator.partition_managed_files(sync_managed)
	if _resolved_targets:
	_resolved_buckets = BaseIntegrator.partition_managed_files(
	sync_managed, targets=_resolved_targets
	)
	for name, paths in _resolved_buckets.items():
	bucket = _buckets.setdefault(name, builtins.set())
	bucket.update(paths)

[Copilot]

This orphan cleanup always calls validate_deploy_path() with the default prefix allow-list (KNOWN_TARGETS). In user scope, lockfile paths will now legitimately include prefixes like .copilot/ and .config/opencode/, so they will be rejected and not cleaned up. Consider passing the current resolved targets (and optionally merging with default prefixes for legacy .github/ paths) so orphaned user-scope files/directories are actually removed.

See below for a potential fix:

            # Derive an allow-list of prefixes from the orphaned paths themselves,
            # so user-scope prefixes (e.g., .copilot/, .config/opencode/) are
            # considered valid for cleanup. Also include legacy .github paths.
            _orphan_allowed_prefixes: builtins.set = set()
            for _p in orphaned_deployed_files:
                _p_str = str(_p)
                _first_segment = _p_str.split("/", 1)[0] if _p_str else ""
                if _first_segment:
                    _orphan_allowed_prefixes.add(_first_segment)
            if GITHUB_DIR:
                _orphan_allowed_prefixes.add(GITHUB_DIR.strip("/"))
            for _orphan_path in sorted(orphaned_deployed_files):
                # validate_deploy_path() is the safety gate: it rejects path-traversal,
                # requires an allowed integration prefix, and checks the resolved path
                # stays within project_root -- so rmtree is safe here.
                # Pass the prefixes derived from the current orphan set so that
                # user-scope paths are also cleaned up, while still honoring
                # legacy .github deployments.
                if BaseIntegrator.validate_deploy_path(
                    _orphan_path,
                    project_root,
                    allowed_prefixes=_orphan_allowed_prefixes,
                ):

[Copilot]

error_msg is assigned but never used. To avoid unused-variable noise and clarify intent, consider using _ here (or logging/recording the validation error if it's meant to be surfaced).

Suggested change

	is_valid, error_msg = validate_skill_name(raw_name)
	is_valid, _ = validate_skill_name(raw_name)

[Copilot]

These changes add multiple bullet entries for the same PR number (#542) under ### Fixed. The repo's changelog convention is one line per PR; consider collapsing these into a single concise bullet (and keep the remaining detail in the PR description) to match the established format.

[danielmeppiel]

@copilot assess if latest unresolved comments are legit and propose engancements in a separate PR if helpful (against main)