-
Notifications
You must be signed in to change notification settings - Fork 186
fix: scope-resolved target profiles + Claude Code instructions #542
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.
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
Changes from all commits
7036e5a
9b5934c
0656398
7d2ffb3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1011,27 +1011,10 @@ def _log_integration(msg): | |
|
|
||
| # --- target x primitive dispatch loop --- | ||
| for _target in targets: | ||
| # Skip entire target when user scope is not supported | ||
| if _user_scope and not _target.user_supported: | ||
| if logger: | ||
| logger.verbose_detail( | ||
| f"Skipping {_target.name} at user scope (not supported)" | ||
| ) | ||
| continue | ||
|
|
||
| for _prim_name, _mapping in _target.primitives.items(): | ||
| if _prim_name == "skills": | ||
| continue # handled separately below | ||
|
|
||
| # Skip primitives unsupported at user scope | ||
| if _user_scope and _prim_name in _target.unsupported_user_primitives: | ||
| if logger: | ||
| logger.verbose_detail( | ||
| f"Skipping {_prim_name} for {_target.name} " | ||
| f"at user scope (not supported)" | ||
| ) | ||
| continue | ||
|
|
||
| # --- hooks (different return type) --- | ||
| if _prim_name == "hooks": | ||
| hook_result = hook_integrator.integrate_hooks_for_target( | ||
|
|
@@ -1331,7 +1314,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: | ||
|
|
@@ -1438,22 +1421,18 @@ def _collect_descendants(node, visited=None): | |
| # Determine active targets. When --target or apm.yml target is set | ||
| # the user's choice wins. Otherwise auto-detect from existing dirs, | ||
| # falling back to copilot when nothing is found. | ||
| from apm_cli.integration.targets import ( | ||
| active_targets as _active_targets, | ||
| active_targets_user_scope as _active_targets_user, | ||
| ) | ||
| from apm_cli.integration.targets import resolve_targets as _resolve_targets | ||
|
|
||
| _is_user = scope is InstallScope.USER | ||
| if _is_user: | ||
| _targets = _active_targets_user(explicit_target=_explicit) | ||
| else: | ||
| _targets = _active_targets(project_root, explicit_target=_explicit) | ||
| _targets = _resolve_targets( | ||
| project_root, user_scope=_is_user, explicit_target=_explicit, | ||
| ) | ||
|
|
||
| # Log target detection results | ||
| if logger and _targets: | ||
| _scope_label = "global" if _is_user else "project" | ||
| _target_names = ", ".join( | ||
| f"{t.name} (~/{t.effective_root(user_scope=_is_user)}/)" | ||
| f"{t.name} (~/{t.root_dir}/)" | ||
| if _is_user else t.name | ||
| for t in _targets | ||
| ) | ||
|
|
@@ -1467,7 +1446,7 @@ def _collect_descendants(node, visited=None): | |
| ) | ||
|
|
||
| for _t in _targets: | ||
| _root = _t.effective_root(user_scope=_is_user) | ||
| _root = _t.root_dir | ||
| _target_dir = project_root / _root | ||
| if not _target_dir.exists(): | ||
| _target_dir.mkdir(parents=True, exist_ok=True) | ||
|
|
@@ -2357,8 +2336,10 @@ def _collect_descendants(node, visited=None): | |
| _deleted_orphan_paths: builtins.list = [] | ||
| for _orphan_path in sorted(orphaned_deployed_files): | ||
| # 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. | ||
| # requires a known integration prefix, and checks the resolved path | ||
| # stays within project_root -- so rmtree is safe here. | ||
| # Use default prefixes (all KNOWN_TARGETS) so legacy deployed | ||
| # paths from older buggy installs are also cleaned up. | ||
| if BaseIntegrator.validate_deploy_path(_orphan_path, project_root): | ||
|
Comment on lines
+2339
to
2343
|
||
| _target = project_root / _orphan_path | ||
|
danielmeppiel marked this conversation as resolved.
|
||
| if _target.exists(): | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -224,8 +224,12 @@ 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): | ||||||||||||||||||||||||||||
| """Remove deployed files and re-integrate from remaining packages.""" | ||||||||||||||||||||||||||||
| 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. | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| When *user_scope* is ``True``, targets are resolved for user-level | ||||||||||||||||||||||||||||
| deployment so cleanup and re-integration use the correct paths. | ||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||
| from ...integration.base_integrator import BaseIntegrator | ||||||||||||||||||||||||||||
| from ...models.apm_package import PackageInfo, validate_apm_package | ||||||||||||||||||||||||||||
| from ...integration.prompt_integrator import PromptIntegrator | ||||||||||||||||||||||||||||
|
|
@@ -234,10 +238,19 @@ def _sync_integrations_after_uninstall(apm_package, project_root, all_deployed_f | |||||||||||||||||||||||||||
| from ...integration.command_integrator import CommandIntegrator | ||||||||||||||||||||||||||||
| from ...integration.hook_integrator import HookIntegrator | ||||||||||||||||||||||||||||
| from ...integration.instruction_integrator import InstructionIntegrator | ||||||||||||||||||||||||||||
| from ...integration.targets import KNOWN_TARGETS, active_targets | ||||||||||||||||||||||||||||
| from ...integration.targets import resolve_targets | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| # Resolve targets once -- used for both Phase 1 removal and Phase 2 re-integration. | ||||||||||||||||||||||||||||
| config_target = apm_package.target | ||||||||||||||||||||||||||||
| _explicit = config_target or None | ||||||||||||||||||||||||||||
| _resolved_targets = resolve_targets(project_root, user_scope=user_scope, explicit_target=_explicit) | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| sync_managed = all_deployed_files if all_deployed_files else None | ||||||||||||||||||||||||||||
| if sync_managed is not None: | ||||||||||||||||||||||||||||
| # For removal, partition against both resolved AND unresolved targets | ||||||||||||||||||||||||||||
| # so legacy deployed_files (from older buggy user-scope installs that | ||||||||||||||||||||||||||||
| # wrote to .github/ instead of .copilot/) are still bucketed and | ||||||||||||||||||||||||||||
| # cleaned up during uninstall. | ||||||||||||||||||||||||||||
| _buckets = BaseIntegrator.partition_managed_files(sync_managed) | ||||||||||||||||||||||||||||
|
Comment on lines
+253
to
254
|
||||||||||||||||||||||||||||
| # 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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.