refactor: unify integration dispatch, result types, and hook dedup

[danielmeppiel]

Motivation

APM's integration layer had primitive-to-integrator mappings scattered across
three separate dispatch tables in install.py and engine.py, hooks excluded
from all of them, and three different result types that callers had to
destructure with type-specific field names. Every new primitive or hook target
required edits in 3+ locations, and forgetting one silently broke uninstall.
This refactor consolidates everything into a single dispatch table with a
unified result type so that adding a primitive is one dict entry and the test
suite catches omissions automatically.

Before / After Summary

Aspect	Before	After
Dispatch tables	3 (_PRIMITIVE_INTEGRATORS, _SYNC_DISPATCH, _REINT_DISPATCH)	1 (PRIMITIVE_DISPATCH in dispatch.py)
Hook dispatch	Inline if target.name branches (3 copies)	In the table + _MERGE_HOOK_TARGETS config dict
Result types	3 (IntegrationResult, HookIntegrationResult, SkillIntegrationResult)	1 (IntegrationResult with optional fields)
Adding a primitive	Edit 3 files, 3 dicts	1 entry in dispatch.py
Adding a merge-hook target	Copy-paste a method	1 entry in _MERGE_HOOK_TARGETS
Wiring validation	Manual review	check_primitive_coverage() auto-runs in test suite

Before: 3-Way Dispatch

flowchart TD
    A[install.py] --> B{primitive type?}
    B -->|prompts/agents/cmds/instr| C[_PRIMITIVE_INTEGRATORS table]
    B -->|hooks| D[Inline if branch]
    B -->|skills| E[Separate call block]
    C --> F[IntegrationResult]
    D --> G[HookIntegrationResult]
    E --> H[SkillIntegrationResult]
    F --> I[Type-specific field access]
    G --> I
    H --> I

Loading

After: Unified Dispatch

flowchart TD
    A["install.py / engine.py"] --> B["get_dispatch_table()"]
    B --> C["PRIMITIVE_DISPATCH (dispatch.py)"]
    C --> D{multi_target?}
    D -->|No| E["Per-target loop: integrator.*_for_target()"]
    D -->|Yes skills| F["All-target: integrate_package_skill()"]
    E --> G["IntegrationResult (unified)"]
    F --> G
    G --> H["Uniform field access: files_integrated, target_paths"]

Loading

Improvement Details

1. Single dispatch table (dispatch.py)

A new PrimitiveDispatch frozen dataclass maps each primitive name to its
integrator class, install method, uninstall method, counter key, and a
multi_target flag. The get_dispatch_table() function builds the table lazily
on first access (deferred imports avoid circular dependencies). Both install.py
and engine.py import and iterate the same table.

2. Unified IntegrationResult

The three result dataclasses (IntegrationResult, HookIntegrationResult,
SkillIntegrationResult) are replaced by a single IntegrationResult with
shared fields (files_integrated, target_paths) and optional fields that
default to zero (scripts_copied, sub_skills_promoted, skill_created).
Callers no longer need type-specific destructuring.

3. Hooks in the dispatch table

Hooks now have a PrimitiveDispatch entry like every other primitive. The
per-target merge logic (Claude/Cursor/Codex) is consolidated into
_integrate_merged_hooks(), driven by a _MERGE_HOOK_TARGETS config dict.
Adding a new JSON-merge hook target (e.g., Windsurf) is one dict entry -- no
method duplication required.

4. _MERGE_HOOK_TARGETS config dict

Each merge-hook target is described by a _MergeHookConfig (config filename,
target key, require_dir flag). The shared _integrate_merged_hooks() method
handles file discovery, JSON merging, _apm_source tagging, and script copying
for all targets uniformly. The three near-identical per-target methods now
delegate to this single implementation.

5. check_primitive_coverage() safety net

A new coverage.py module exposes check_primitive_coverage(), which compares
KNOWN_TARGETS primitives against dispatch table keys. A conftest.py
session-scoped fixture runs this check at test startup, so a missing dispatch
entry fails the entire test suite immediately -- no silent breakage.

Command Coverage

Command	Uses dispatch table?	Notes
apm install	Yes	Per-target + multi-target dispatch loop
apm uninstall	Yes	sync_method from same dispatch entries
apm deps update	Yes	Delegates to _install_apm_dependencies(update_refs=True), same loop
apm deps list	No	Read-only lockfile query, no integration
apm deps tree	No	Read-only dependency tree, no integration
apm deps info	No	Read-only package metadata, no integration

Testing

• Coverage assertion: check_primitive_coverage() runs as a session-scoped
  autouse fixture in conftest.py. Tests a happy path (all primitives covered)
  and a negative path (missing primitive raises RuntimeError).
• Dispatch table validation: test_data_driven_dispatch.py validates that
  every entry in the dispatch table has valid method names, correct counter keys,
  and that multi_target flags are consistent.
• Hook integration: test_hook_integrator.py covers Claude, Cursor, and
  Codex merge paths through _integrate_merged_hooks(), including _apm_source
  tagging, user-hook preservation, and require_dir skip logic.
• 120 integration tests covering dispatch, hooks, manifests, and individual
  integrators across the test suite.

Stats

• 17 files changed (excluding lockfile), +537 / -403 lines
• 2 new modules: dispatch.py (72 lines), coverage.py (40 lines)
• 120 integration tests collected

[Copilot]

Pull request overview

Refactors APM's integration targeting system to use a single primitive dispatch registry and a unified IntegrationResult, while reducing duplication in hook integration and adding automated coverage checks to prevent dispatch/target drift.

Changes:

• Introduces src/apm_cli/integration/dispatch.py as the single source of truth for primitive integration/sync dispatch (used by install + uninstall).
• Unifies hook results into IntegrationResult and deduplicates Claude/Cursor/Codex hook-merge logic via a shared implementation + config map.
• Adds primitive coverage validation (coverage.py) and wires it into the unit test session to fail fast when KNOWN_TARGETS and dispatch drift.

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/apm_cli/integration/dispatch.py	New unified primitive dispatch registry with lazy initialization.
src/apm_cli/integration/coverage.py	New validation helper to ensure every KNOWN_TARGETS primitive has a dispatch handler.
src/apm_cli/commands/install.py	Replaces local primitive mapping with unified dispatch table + shared counters/logging path.
src/apm_cli/commands/uninstall/engine.py	Uses unified dispatch for phase-1 sync and phase-2 best-effort re-integration.
src/apm_cli/integration/base_integrator.py	Expands IntegrationResult to include optional hook/skill fields.
src/apm_cli/integration/hook_integrator.py	Replaces HookIntegrationResult dataclass with IntegrationResult, adds merged-hook implementation shared across targets.
src/apm_cli/integration/skill_integrator.py	Documents SkillIntegrationResult as deprecated in favor of IntegrationResult for new code.
src/apm_cli/integration/{prompt,command,instruction,agent}_integrator.py	Adds/clarifies DEPRECATED legacy per-target wrapper guidance.
src/apm_cli/integration/__init__.py	Exposes new dispatch/coverage utilities from the integration package.
tests/conftest.py	Adds session-scoped autouse primitive coverage validation.
tests/unit/integration/test_*.py	Updates tests to use files_integrated and adds coverage/dispatch table validation tests.

[Copilot]

Pull request overview

This PR refactors APM's integration layer to make primitive integration and uninstall routing table-driven from a single shared dispatch registry, while unifying integrator result types and deduplicating hook merge logic across targets.

Changes:

• Introduces a single primitive dispatch table (get_dispatch_table()) used by both install and uninstall flows, plus a coverage check to prevent missing wiring.
• Unifies integrator result reporting on IntegrationResult (with optional hook/skill fields) and updates tests accordingly.
• Deduplicates merged-hook targets (Claude/Cursor/Codex) via a shared _integrate_merged_hooks() implementation driven by a config dict.

Show a summary per file

File	Description
WIP/targeting-architecture.md	New architecture write-up documenting the target/primitive model and unified dispatch approach.
tests/conftest.py	Adds a session-scoped autouse fixture to fail fast when primitive coverage is incomplete.
tests/unit/integration/test_scope_integration.py	New integration tests validating scope-resolved target behavior with real integrators.
tests/unit/integration/test_scope_install_uninstall.py	New install-then-uninstall cycle tests across targets/scopes (currently uses a monkeypatch workaround).
tests/unit/integration/test_hook_integrator.py	Updates assertions from hooks_integrated to unified files_integrated.
tests/unit/integration/test_deployed_files_manifest.py	Updates hook result assertions to unified files_integrated.
tests/unit/integration/test_data_driven_dispatch.py	Updates exhaustiveness checks to use unified dispatch and adds coverage/dispatch-table validation tests.
tests/unit/integration/test_command_integrator.py	Updates mock result shape to match unified IntegrationResult.
src/apm_cli/integration/base_integrator.py	Extends IntegrationResult with optional hook/skill fields and updates its docstring.
src/apm_cli/integration/hook_integrator.py	Adds _MERGE_HOOK_TARGETS + _integrate_merged_hooks() and introduces a backward-compat HookIntegrationResult shim.
src/apm_cli/integration/dispatch.py	New unified primitive dispatch registry with lazy initialization.
src/apm_cli/integration/coverage.py	New bidirectional primitive coverage validation helper.
src/apm_cli/integration/init.py	Exposes dispatch/coverage utilities at the integration package level.
src/apm_cli/integration/prompt_integrator.py	Clarifies deprecated legacy per-target APIs vs target-driven API.
src/apm_cli/integration/agent_integrator.py	Clarifies deprecated legacy per-target APIs vs target-driven API.
src/apm_cli/integration/instruction_integrator.py	Clarifies deprecated legacy per-target APIs vs target-driven API.
src/apm_cli/integration/command_integrator.py	Clarifies deprecated legacy per-target APIs vs target-driven API.
src/apm_cli/integration/skill_integrator.py	Marks legacy SkillIntegrationResult as deprecated in favor of IntegrationResult.
src/apm_cli/commands/install.py	Replaces local primitive dispatch logic with get_dispatch_table() driven dispatch.
src/apm_cli/commands/uninstall/engine.py	Replaces sync/reintegration dispatch tables with unified dispatch-table driven logic.
src/apm_cli/commands/deps/cli.py	Fixes apm deps update -g by passing scope through to the install path.
CHANGELOG.md	Adds Unreleased entries describing the refactor and the deps update -g scope fix.

Copilot's findings

Comments suppressed due to low confidence (2)

src/apm_cli/integration/hook_integrator.py:551

• The deprecated per-target methods (integrate_package_hooks_claude/cursor/codex) declare they return HookIntegrationResult, but they delegate to _integrate_merged_hooks() which returns IntegrationResult. That means callers using the legacy hooks_integrated field/property will break even though these APIs are marked as backward-compatible. Wrap the returned IntegrationResult in HookIntegrationResult (or have _integrate_merged_hooks optionally produce the shim) so deprecated methods preserve the old surface area.

    def integrate_package_hooks_claude(self, package_info, project_root: Path,
                                        force: bool = False,
                                        managed_files: set = None,
                                        diagnostics=None) -> HookIntegrationResult:
        """Integrate hooks into .claude/settings.json.

        .. deprecated:: Use :meth:`integrate_hooks_for_target` instead.
        """
        return self._integrate_merged_hooks(
            _MERGE_HOOK_TARGETS["claude"], package_info, project_root,
            force=force, managed_files=managed_files,
            diagnostics=diagnostics,
        )

src/apm_cli/integration/hook_integrator.py:602

• integrate_hooks_for_target() is annotated to return HookIntegrationResult, but in practice it returns IntegrationResult (from integrate_package_hooks(), _integrate_merged_hooks(), and the final fallback). Update the return type to IntegrationResult (unified result type) or consistently return the HookIntegrationResult shim if you want the legacy alias to remain available.

    def integrate_hooks_for_target(
        self,
        target,
        package_info,
        project_root: Path,
        *,
        force: bool = False,
        managed_files: set = None,
        diagnostics=None,
    ) -> "HookIntegrationResult":
        """Integrate hooks for a single *target*.

        Copilot uses individual JSON files (genuinely different pattern).
        All other merge-based targets are dispatched via the
        ``_MERGE_HOOK_TARGETS`` registry.
        """
        if target.name == "copilot":
            return self.integrate_package_hooks(

• Files reviewed: 22/23 changed files
• Comments generated: 6

[Copilot]

integrate_package_hooks() is still annotated to return HookIntegrationResult, but the no-hook early return now returns an IntegrationResult. If external consumers still call this legacy API and expect the hooks_integrated alias/property, this is a breaking change. Consider returning HookIntegrationResult here (wrapping the unified IntegrationResult), or update the method signature/docstring to reflect the new return type and explicitly document the breaking change/deprecation behavior.

This issue also appears in the following locations of the same file:

• line 539
• line 585

[Copilot]

In Phase 1 user-scope uninstall, sync_for_target() ultimately calls BaseIntegrator.sync_remove_files(), which calls validate_deploy_path(rel_path, project_root) without passing scope-resolved targets/allowed_prefixes. That validation falls back to KNOWN_TARGETS prefixes (project-scope roots) and will reject user-scope paths like .copilot/... and .config/opencode/..., so those files will be silently skipped and left behind. Please plumb the resolved targets (or an allowlist derived from them) through the sync_remove_files/validate_deploy_path call chain so user-scope managed files can be removed without test monkeypatching.

[Copilot]

_integrate_package_primitives() imports InstallScope but no longer uses it (the previous _user_scope computation was removed). This will fail linting/unused-import checks; please remove the unused import (and consider dropping the now-unused scope parameter if it's truly no longer needed).

Suggested change

from apm_cli.core.scope import InstallScope

[Copilot]

This test monkeypatches BaseIntegrator._get_integration_prefixes to allow user-scope prefixes, which indicates uninstall cleanup is currently dependent on the allowlist produced from KNOWN_TARGETS (project-scope roots). This makes the cycle tests diverge from production behavior. Prefer fixing the production sync/validate_deploy_path plumbing so user-scope uninstall works without monkeypatching, then remove this fixture and the hardcoded _USER_SCOPE_PREFIXES.

[Copilot]

This PR fixes apm deps update -g scope behavior, but the user-facing docs still don't list a global flag for apm deps update (e.g., docs/src/content/docs/reference/cli-commands.md and packages/apm-guide/.apm/skills/apm-usage/commands.md only list verbose/force/target/parallel-downloads). Please update the Starlight docs pages and the apm-guide command reference to document the global/user-scope behavior for deps update (and any related scope semantics impacted by the unified dispatch change).

[Copilot]

counts is hardcoded to the current primitive set. One of the PR goals is that adding a new primitive is a single dispatch-table change; with this hardcoded dict, adding a new dispatch entry/primitive will cause KeyError or missing accounting unless this is manually updated too. Consider deriving the counters dict from _dispatch (e.g., {entry.counter_key: 0 for entry in _dispatch.values()}) to keep uninstall aligned with the unified registry.

Suggested change

	counts = {"prompts": 0, "agents": 0, "skills": 0, "commands": 0, "hooks": 0, "instructions": 0}
	counts = {entry.counter_key: 0 for entry in _dispatch.values()}