fix: scope-aware hook deployment, dynamic sync prefixes, auto_create guard (#566)

sergio-sisternes-epam · danielmeppiel · Copilot · web-flow · commit 3752b6074f63 · 2026-04-06T23:27:09.000+02:00
Rework of PR #566 on top of post-#562 architecture: - integrate_package_hooks: accept target=, use target.root_dir instead of hardcoded .github for Copilot hook deployment path - _integrate_merged_hooks: use target.root_dir instead of f'.{config.target_key}' for Claude/Cursor/Codex JSON merge path - _rewrite_command_for_target / _rewrite_hooks_data: accept root_dir= override so script paths match the scope-resolved directory - sync_integration: derive hook prefixes dynamically from KNOWN_TARGETS instead of hardcoded .github/.claude/.cursor/.codex paths; accept targets= parameter - install.py: gate directory creation on auto_create=True to prevent creating dirs for targets that don't want them (e.g. Claude, Cursor at project scope) - 6 new tests covering scope-resolved deploy, script rewrite, sync cleanup, and auto_create guard Fixes #565 Addresses #576 (P1-1 sync, P1-4 auto_create) Co-authored-by: danielmeppiel <dmeppiel@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- `apm install -g` now deploys hooks to the scope-resolved target directory (e.g. `~/.copilot/hooks/`) instead of hardcoding `.github/hooks/` (#565)
+- Hook sync/cleanup now derives prefixes dynamically from `KNOWN_TARGETS` instead of hardcoded paths (#565)
+- `auto_create=False` targets no longer get directories unconditionally created during install (#576)
 - `apm deps update -g` now correctly passes scope, preventing user-scope updates from silently using project-scope paths (#562)
 
 ## [0.8.10] - 2026-04-03
diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py
@@ -1431,6 +1431,8 @@ def _collect_descendants(node, visited=None):
                 )
 
         for _t in _targets:
+            if not _t.auto_create:
+                continue
             _root = _t.root_dir
             _target_dir = project_root / _root
             if not _target_dir.exists():
diff --git a/src/apm_cli/integration/hook_integrator.py b/src/apm_cli/integration/hook_integrator.py
@@ -192,6 +192,7 @@ def _rewrite_command_for_target(
         package_name: str,
         target: str,
         hook_file_dir: Optional[Path] = None,
+        root_dir: Optional[str] = None,
     ) -> Tuple[str, List[Tuple[Path, str]]]:
         """Rewrite a hook command to use installed script paths.
 
@@ -205,6 +206,7 @@ def _rewrite_command_for_target(
             package_name: Name used for the scripts subdirectory
             target: "vscode" or "claude"
             hook_file_dir: Directory containing the hook JSON file (for ./path resolution)
+            root_dir: Override root directory (e.g. ".copilot" for user scope)
 
         Returns:
             Tuple of (rewritten_command, list of (source_file, relative_target_path))
@@ -213,13 +215,17 @@ def _rewrite_command_for_target(
         new_command = command
 
         if target == "vscode":
-            scripts_base = f".github/hooks/scripts/{package_name}"
+            base_root = root_dir or ".github"
+            scripts_base = f"{base_root}/hooks/scripts/{package_name}"
         elif target == "cursor":
-            scripts_base = f".cursor/hooks/{package_name}"
+            base_root = root_dir or ".cursor"
+            scripts_base = f"{base_root}/hooks/{package_name}"
         elif target == "codex":
-            scripts_base = f".codex/hooks/{package_name}"
+            base_root = root_dir or ".codex"
+            scripts_base = f"{base_root}/hooks/{package_name}"
         else:
-            scripts_base = f".claude/hooks/{package_name}"
+            base_root = root_dir or ".claude"
+            scripts_base = f"{base_root}/hooks/{package_name}"
 
         # Handle ${CLAUDE_PLUGIN_ROOT} references (always relative to package root)
         plugin_root_pattern = r'\$\{CLAUDE_PLUGIN_ROOT\}(/[^\s]+)'
@@ -263,6 +269,7 @@ def _rewrite_hooks_data(
         package_name: str,
         target: str,
         hook_file_dir: Optional[Path] = None,
+        root_dir: Optional[str] = None,
     ) -> Tuple[Dict, List[Tuple[Path, str]]]:
         """Rewrite all command paths in a hooks JSON structure.
 
@@ -274,6 +281,7 @@ def _rewrite_hooks_data(
             package_name: Name for scripts subdirectory
             target: "vscode" or "claude"
             hook_file_dir: Directory containing the hook JSON file (for ./path resolution)
+            root_dir: Override root directory (e.g. ".copilot" for user scope)
 
         Returns:
             Tuple of (rewritten_data_copy, list of (source_file, target_rel_path))
@@ -296,6 +304,7 @@ def _rewrite_hooks_data(
                         new_cmd, scripts = self._rewrite_command_for_target(
                             matcher[key], package_path, package_name, target,
                             hook_file_dir=hook_file_dir,
+                            root_dir=root_dir,
                         )
                         if scripts:
                             _log.debug(
@@ -315,6 +324,7 @@ def _rewrite_hooks_data(
                             new_cmd, scripts = self._rewrite_command_for_target(
                                 hook[key], package_path, package_name, target,
                                 hook_file_dir=hook_file_dir,
+                                root_dir=root_dir,
                             )
                             if scripts:
                                 _log.debug(
@@ -348,8 +358,9 @@ def _get_package_name(self, package_info) -> str:
     def integrate_package_hooks(self, package_info, project_root: Path,
                                  force: bool = False,
                                  managed_files: set = None,
-                                 diagnostics=None) -> HookIntegrationResult:
-        """Integrate hooks from a package into .github/hooks/ (VSCode target).
+                                 diagnostics=None,
+                                 target=None) -> HookIntegrationResult:
+        """Integrate hooks from a package into hooks dir (Copilot target).
 
         Deploys hook JSON files with clean filenames and copies referenced
         script files. Skips user-authored files unless force=True.
@@ -359,6 +370,7 @@ def integrate_package_hooks(self, package_info, project_root: Path,
             project_root: Root directory of the project
             force: If True, overwrite user-authored files on collision
             managed_files: Set of relative paths known to be APM-managed
+            target: Optional TargetProfile for scope-resolved root_dir
 
         Returns:
             HookIntegrationResult: Results of the integration operation
@@ -371,7 +383,8 @@ def integrate_package_hooks(self, package_info, project_root: Path,
                 files_skipped=0, target_paths=[],
             )
 
-        hooks_dir = project_root / ".github" / "hooks"
+        root_dir = target.root_dir if target else ".github"
+        hooks_dir = project_root / root_dir / "hooks"
         hooks_dir.mkdir(parents=True, exist_ok=True)
 
         package_name = self._get_package_name(package_info)
@@ -388,6 +401,7 @@ def integrate_package_hooks(self, package_info, project_root: Path,
             rewritten, scripts = self._rewrite_hooks_data(
                 data, package_info.install_path, package_name, "vscode",
                 hook_file_dir=hook_file.parent,
+                root_dir=root_dir,
             )
 
             # Generate target filename (clean, no -apm suffix)
@@ -436,6 +450,7 @@ def _integrate_merged_hooks(
         force: bool = False,
         managed_files: set = None,
         diagnostics=None,
+        target=None,
     ) -> HookIntegrationResult:
         """Integrate hooks by merging into a target-specific JSON config.
 
@@ -448,7 +463,8 @@ def _integrate_merged_hooks(
             files_skipped=0, target_paths=[],
         )
 
-        target_dir = project_root / f".{config.target_key}"
+        root_dir = target.root_dir if target else f".{config.target_key}"
+        target_dir = project_root / root_dir
 
         # Opt-in check: some targets only deploy when their dir exists
         if config.require_dir and not target_dir.exists():
@@ -486,6 +502,7 @@ def _integrate_merged_hooks(
                 data, package_info.install_path, package_name,
                 config.target_key,
                 hook_file_dir=hook_file.parent,
+                root_dir=root_dir,
             )
 
             # Merge hooks into config (additive)
@@ -603,6 +620,7 @@ def integrate_hooks_for_target(
                 package_info, project_root,
                 force=force, managed_files=managed_files,
                 diagnostics=diagnostics,
+                target=target,
             )
 
         config = _MERGE_HOOK_TARGETS.get(target.name)
@@ -611,6 +629,7 @@ def integrate_hooks_for_target(
                 config, package_info, project_root,
                 force=force, managed_files=managed_files,
                 diagnostics=diagnostics,
+                target=target,
             )
 
         return HookIntegrationResult(
@@ -619,7 +638,8 @@ def integrate_hooks_for_target(
         )
 
     def sync_integration(self, apm_package, project_root: Path,
-                          managed_files: set = None) -> Dict:
+                          managed_files: set = None,
+                          targets=None) -> Dict:
         """Remove APM-managed hook files.
 
         Uses *managed_files* (relative paths) to surgically remove only
@@ -628,35 +648,41 @@ def sync_integration(self, apm_package, project_root: Path,
 
         **Never** calls ``shutil.rmtree``.
 
-        Also cleans APM entries from ``.claude/settings.json`` and
-        ``.cursor/hooks.json`` via the ``_apm_source`` marker.
+        Also cleans APM entries from merged-hook JSON files via the
+        ``_apm_source`` marker.
         """
+        from .targets import KNOWN_TARGETS
+
         stats: Dict[str, int] = {'files_removed': 0, 'errors': 0}
 
+        # Derive hook prefixes dynamically from targets
+        source = targets if targets is not None else list(KNOWN_TARGETS.values())
+        hook_prefixes = []
+        for t in source:
+            if t.supports("hooks"):
+                sm = t.primitives["hooks"]
+                effective_root = sm.deploy_root or t.root_dir
+                hook_prefixes.append(f"{effective_root}/hooks/")
+        hook_prefix_tuple = tuple(hook_prefixes)
+
         if managed_files is not None:
-            # Manifest-based removal  -- only remove tracked files
+            # Manifest-based removal -- only remove tracked files
             deleted: list = []
             for rel_path in managed_files:
-                # Normalize path separators for cross-platform compatibility
                 normalized = rel_path.replace("\\", "/")
-                # Only handle hook-related paths
-                is_hook = (
-                    normalized.startswith(".github/hooks/")
-                    or normalized.startswith(".claude/hooks/")
-                    or normalized.startswith(".cursor/hooks/")
-                    or normalized.startswith(".codex/hooks/")
-                )
-                if not is_hook or ".." in rel_path:
+                if not normalized.startswith(hook_prefix_tuple):
+                    continue
+                if ".." in rel_path:
                     continue
-                target = project_root / rel_path
-                if target.exists() and target.is_file():
+                target_file = project_root / rel_path
+                if target_file.exists() and target_file.is_file():
                     try:
-                        target.unlink()
+                        target_file.unlink()
                         stats['files_removed'] += 1
-                        deleted.append(target)
+                        deleted.append(target_file)
                     except Exception:
                         stats['errors'] += 1
-            # Batch parent cleanup  -- single bottom-up pass
+            # Batch parent cleanup -- single bottom-up pass
             self.cleanup_empty_parents(deleted, stop_at=project_root)
         else:
             # Legacy fallback  -- glob for old -apm suffix files
@@ -669,48 +695,45 @@ def sync_integration(self, apm_package, project_root: Path,
                     except Exception:
                         stats['errors'] += 1
 
-        # Clean APM entries from .claude/settings.json (safe  -- uses _apm_source marker)
-        settings_path = project_root / ".claude" / "settings.json"
-        if settings_path.exists():
-            try:
-                with open(settings_path, 'r', encoding='utf-8') as f:
-                    settings = json.load(f)
-
-                if "hooks" in settings:
-                    modified = False
-                    for event_name in list(settings["hooks"].keys()):
-                        matchers = settings["hooks"][event_name]
-                        if isinstance(matchers, list):
-                            filtered = [
-                                m for m in matchers
-                                if not (isinstance(m, dict) and "_apm_source" in m)
-                            ]
-                            if len(filtered) != len(matchers):
-                                modified = True
-                            settings["hooks"][event_name] = filtered
-                            if not filtered:
-                                del settings["hooks"][event_name]
-
-                    if not settings["hooks"]:
-                        del settings["hooks"]
-
-                    if modified:
-                        with open(settings_path, 'w', encoding='utf-8') as f:
-                            json.dump(settings, f, indent=2)
-                            f.write('\n')
-                        stats['files_removed'] += 1
-            except (json.JSONDecodeError, OSError):
-                stats['errors'] += 1
-
-        # Clean APM entries from .cursor/hooks.json (safe  -- uses _apm_source marker)
-        self._clean_apm_entries_from_json(
-            project_root / ".cursor" / "hooks.json", stats,
-        )
-
-        # Clean APM entries from .codex/hooks.json (safe  -- uses _apm_source marker)
-        self._clean_apm_entries_from_json(
-            project_root / ".codex" / "hooks.json", stats,
-        )
+        # Clean APM entries from merged-hook JSON configs (uses _apm_source marker)
+        for t in source:
+            config = _MERGE_HOOK_TARGETS.get(t.name)
+            if config is not None:
+                json_path = project_root / t.root_dir / config.config_filename
+                if t.name == "claude":
+                    # Claude uses settings.json with special structure
+                    if json_path.exists():
+                        try:
+                            with open(json_path, 'r', encoding='utf-8') as f:
+                                settings = json.load(f)
+
+                            if "hooks" in settings:
+                                modified = False
+                                for event_name in list(settings["hooks"].keys()):
+                                    matchers = settings["hooks"][event_name]
+                                    if isinstance(matchers, list):
+                                        filtered = [
+                                            m for m in matchers
+                                            if not (isinstance(m, dict) and "_apm_source" in m)
+                                        ]
+                                        if len(filtered) != len(matchers):
+                                            modified = True
+                                        settings["hooks"][event_name] = filtered
+                                        if not filtered:
+                                            del settings["hooks"][event_name]
+
+                                if not settings["hooks"]:
+                                    del settings["hooks"]
+
+                                if modified:
+                                    with open(json_path, 'w', encoding='utf-8') as f:
+                                        json.dump(settings, f, indent=2)
+                                        f.write('\n')
+                                    stats['files_removed'] += 1
+                        except (json.JSONDecodeError, OSError):
+                            stats['errors'] += 1
+                else:
+                    self._clean_apm_entries_from_json(json_path, stats)
 
         return stats
 
diff --git a/tests/unit/integration/test_hook_integrator.py b/tests/unit/integration/test_hook_integrator.py

Original file line number	Diff line number	Diff line change
`@@ -1431,6 +1431,8 @@ def _collect_descendants(node, visited=None):`
`1431`	`1431`	`)`
`1432`	`1432`
`1433`	`1433`	`for _t in _targets:`
	`1434`	`+ if not _t.auto_create:`
	`1435`	`+ continue`
`1434`	`1436`	`_root = _t.root_dir`
`1435`	`1437`	`_target_dir = project_root / _root`
`1436`	`1438`	`if not _target_dir.exists():`