Skip to content

fix: use scope-resolved target dir for hook deployment #566

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
danielmeppiel merged 1 commit into from
Apr 6, 2026
Merged

fix: use scope-resolved target dir for hook deployment #566

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/apm_cli/commands/install.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
159 changes: 91 additions & 68 deletions src/apm_cli/integration/hook_integrator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand All @@ -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))
Expand All @@ -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]+)'
Expand Down Expand Up @@ -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.

Expand All @@ -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))
Expand All @@ -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(
Expand All @@ -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(
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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.

Expand All @@ -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():
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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(
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down
Loading
Loading