-
Notifications
You must be signed in to change notification settings - Fork 7
Description
Context
PR #234 added removeNewlyExcludedComponentArtifacts() and reconcileStaleArtifacts() to handle convergence gaps. Both methods duplicate the same per-artifact-type removal patterns (MCP servers, files, brew packages, plugins, gitignore entries) that already exist in unconfigurePack().
Problem
Three methods in Configurator.swift perform artifact removal with near-identical patterns:
unconfigurePack()— removes all artifacts for a whole packremoveNewlyExcludedComponentArtifacts()— removes artifacts for newly-excluded componentsreconcileStaleArtifacts()— removes stale artifacts from previous sync
Each reimplements: ResourceRefCounter checks for brew/plugins, GitignoreManager removal loops, exec.removeMCPServer() calls, strategy.removeFileArtifact() calls. ~100-120 lines of duplicated removal logic.
Proposed Solution
Extract per-artifact-type removal helpers:
removeMCPArtifact(name:scope:executor:) -> BoolremoveFileArtifact(relativePath:) -> BoolremoveBrewArtifact(package:packID:refCounter:executor:) -> BoolremovePluginArtifact(name:packID:refCounter:executor:) -> BoolremoveGitignoreArtifact(entry:) -> Bool
All three callsites can then use these helpers, reducing duplication and ensuring removal logic stays consistent.
Notes
The three methods operate at different abstraction levels (component definitions vs artifact records), so a full unification isn't practical. Per-type helpers are the right granularity.
Discovered during /simplify code review of PR #234.