Skip to content

Architecture: Unify integration dispatch, result types, and hook dedup #561

@danielmeppiel

Description

@danielmeppiel

Summary

The targeting system works well but has structural debt that raises the cost of adding new targets or primitives -- a critical concern for an OSS project with external contributors who may not know APM internals. This issue tracks 5 improvements ranked by ROI, organized into implementation waves with quality gates.

Full analysis with code examples available in WIP/architecture-improvements.md (local working doc)
Architecture reference available in WIP/targeting-architecture.md (local working doc)


The 5 Improvements

1. Primitive Coverage Assertion (Safety: High | Effort: S)

Problem: KNOWN_TARGETS (registry) and _PRIMITIVE_INTEGRATORS (dispatch table) must stay in sync, but nothing enforces this. A contributor adding a primitive to a target profile but forgetting to wire the integrator gets silent failure -- no error, no warning, just missing files.

Solution: ~15 lines of import-time assertion that computes all primitives from KNOWN_TARGETS and verifies each has a handler in the dispatch table or explicit special-case set ({"skills", "hooks"}). Turns a silent omission into a loud crash during development.

2. Single Result Type (Readability: High | Effort: S)

Problem: Three separate result dataclasses (IntegrationResult, HookIntegrationResult, SkillIntegrationResult) with different field names for the same concept (files_integrated vs hooks_integrated). The dispatch loop must branch on type to extract counts. Every new primitive could invent its own result type.

Solution: Merge into one IntegrationResult with optional fields (scripts_copied, sub_skills_promoted, skill_created). hooks_integrated -> files_integrated. Dispatch loop collapses to a single code path.

3. Deprecate Legacy Per-Target Methods (Readability: High | Effort: S)

Problem: agent_integrator.py has 9 legacy wrapper methods (integrate_package_agents_claude(), sync_integration_cursor(), etc.) that bypass scope resolution and confuse contributors into adding more wrappers instead of using the target-driven API. ~150 lines of dead-weight boilerplate per integrator.

Solution: Add # DEPRECATED markers, a "DO NOT add new per-target methods" comment block, and update docstrings to point to integrate_*_for_target(). Remove wrappers in a future version.

4. Unified Dispatch Table (Extensibility: High | Effort: M)

Problem: The primitive-to-integrator mapping exists in 3 copies: _PRIMITIVE_INTEGRATORS in install.py, _SYNC_DISPATCH in engine.py, _REINT_DISPATCH in engine.py. Plus hooks are excluded from all tables (separate if branch). Adding a new primitive requires edits in 5+ scattered locations.

Solution: Create src/apm_cli/integration/dispatch.py with a PrimitiveDispatch dataclass and a single PRIMITIVE_DISPATCH dict. Hooks join the table (requires #2 first). Install and uninstall both import from one source of truth. Adding a primitive = 1 dict entry.

5. Hook Merge Strategy Extraction (Extensibility: High | Effort: M)

Problem: integrate_package_hooks_claude(), integrate_package_hooks_cursor(), integrate_package_hooks_codex() are ~90% identical (~80 lines each). They all read a JSON config, rewrite hook data, merge with _apm_source markers, copy scripts, write JSON back. Only the config path differs. The Codex method was literally copy-pasted from Cursor.

Solution: Extract shared loop into _integrate_merged_hooks(config). Per-target methods reduce to a _MERGE_TARGETS config dict. Adding a new merge-target = 1 dict entry instead of ~90 lines.


Implementation Plan

Wave 1: Safety Net (2 parallel tasks)

Task Description Files Dependencies
1A Single Result Type base_integrator.py, hook_integrator.py, skill_integrator.py, install.py, engine.py None
1B Primitive Coverage Assertion New integration/coverage.py, conftest.py or __init__.py None

Gate: All 3500+ tests pass. Expert panel (UX, CLI logging, Python architect) validates no behavioral changes.

Wave 2: Cleanup (1 task, can run parallel with Wave 1)

Task Description Files Dependencies
2A Deprecate Legacy Methods All 5 integrator files None

Gate: Documentation-only change, all tests pass. No behavioral changes.

Wave 3: Structural (2 tasks, sequential)

Task Description Files Dependencies
3A Unified Dispatch Table New integration/dispatch.py, install.py, engine.py 1A (hooks need IntegrationResult)
3B Hook Merge Strategy Extraction hook_integrator.py 3A (cleaner after unified table)

Gate: Install/uninstall symmetry validated with integration tests. Expert panel validates architecture. Cross-target tests verify all 5 targets work correctly for install + uninstall at both project and user scope.

Wave 4: Final Verification

Task Description
4A Full test suite (unit + integration)
4B Expert panel final review (UX, logging, architecture)
4C Documentation update (targeting-architecture.md if needed)

Dependency Graph

Wave 1:  1A (Result Type) ----+----> Wave 3: 3A (Dispatch Table) ---> 3B (Hook Dedup)
         1B (Coverage Assert) -+
                                \
Wave 2:  2A (Deprecate Legacy) --+--> (independent, anytime)

Wave 4:  Final verification (after all waves)

Critical path: 1A -> 3A -> 3B (result type unblocks dispatch table, which unblocks hook dedup)

Quality Gate Criteria

After each wave, the following must be verified:

  1. uv run pytest tests/unit tests/test_console.py -x -- all tests pass
  2. UX expert: no user-facing output changes (or intentional improvements documented)
  3. CLI logging expert: no direct _rich_* calls in commands, CommandLogger patterns followed
  4. Python architect: patterns consistent with BaseIntegrator conventions, no integrator-level scope awareness
  5. Integration tests: install + uninstall cycle works for all 5 targets at both scopes
  6. ASCII compliance: no unicode in source or output

Estimated Effort

Wave Tasks Effort Parallelizable?
1 1A + 1B 1.5 days Yes (2 agents)
2 2A 0.5 day Yes (with Wave 1)
3 3A + 3B 4-5 days Sequential (3A then 3B)
4 4A + 4B + 4C 1 day Yes (3 agents)
Total 6 tasks 7-8 days

Agent Team Staffing

Role Count Scope
Implementation agents 2 per wave (parallel tasks) Code changes + unit tests
Integration test agent 1 per gate End-to-end install/uninstall cycle tests
UX expert 1 per gate Validates user-facing behavior
CLI logging expert 1 per gate Validates output conventions
Python architect 1 per gate Validates architectural patterns
Wave leader 1 (coordinator) Dispatches tasks, runs gates, reports to PM

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementDeprecated: use type/feature. Kept for issue history; will be removed in milestone 0.10.0.

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions