Skip to content

InstallerEngine conflates provision and diagnose responsibilities #855

Description

@malpern

Current assessment

InstallerEngine remains a large, sensitive facade for install, repair, uninstall, and system inspection. The underlying maintenance debt is real, but the original standalone extraction plan is no longer the desired implementation strategy.

As of 2026-07-12:

  • InstallerEngine.swift is roughly 1,180 lines, up from the 779 lines cited when this issue was opened.
  • It still connects inspection, planning, recipe execution, repair, and single-action routing.
  • The lazy injectedValidator ?? WizardDependencies.systemValidator fallback remains an initialization-order and testability seam.
  • System evidence and decision-making have already moved toward clearer owners through SystemStateProvider, system validation, and InstallerDecisionPipeline.
  • ADR-043 now explicitly keeps InstallerEngine as the public facade and rejects a broad manager/consolidation rewrite without concrete behavioral justification.

Priority and scope

Deferred architectural debt. Do not schedule this as a standalone cleanup project ahead of user-visible bugs or reliability work.

This issue should guide opportunistic improvements when a real installer bug, reliability problem, testability limitation, or compile-boundary change already requires touching the relevant responsibility.

Desired direction

Preserve the public InstallerEngine facade and reinforce the existing execution model:

probe → snapshot → decide → execute → verify

When implementation work naturally crosses one of these boundaries:

  1. Keep raw system evidence and live probes behind the existing SystemStateProvider / validator ownership.
  2. Keep planning pure in InstallerDecisionPipeline; do not introduce live state reads into planning.
  3. Move touched recipe-execution behavior into a narrowly scoped executor only when that extraction materially improves a concrete fix or its tests.
  4. Replace the lazy global validator fallback with explicit injection when changing that initialization path for a real behavior or testability reason.
  5. Delete redundant forwarding methods and bridges rather than preserving new compatibility layers.
  6. Add boundary/failure-path tests before moving behavior.

Explicit non-goals

  • No broad SystemInspector / InstallRecipeRunner rewrite solely to reduce file length.
  • No generic dependency or effects framework.
  • No behavior-neutral multi-manager reorganization.
  • No large PR combining inspection, planning, execution, and recipe organization.

Activation criteria

Take a narrow slice only when at least one is true:

  • a user-visible installer or repair bug crosses the boundary;
  • duplicated live-state reads cause inconsistent decisions;
  • a failure path cannot be tested without removing global/lazy dependency access;
  • measurements identify a meaningful performance or initialization problem;
  • a module/compile boundary requires ownership to move.

Related: #735, #149 and ADR-043.

Surfaced by holistic codebase review 2026-06-09; rescoped 2026-07-12.

Metadata

Metadata

Assignees

No one assigned

    Labels

    keypathScope: KeyPath product, app, installer, or repository worklarge-refactorneeds-decisionRequires a product or scope decision before implementationon-holdValid work intentionally deferred pending evidence, dependency, or product decisionpost-releasePost-public-1.0 work; not required before stable 1.0 releaserefactorBehavior-preserving structural change; requires concrete justificationtech-debtKnown maintainability cost with current evidencetracking-onlyOpen coordination or roadmap issue; do not implement as one PR

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions