Merged
Conversation
- Add stale artifact reconciliation after installArtifacts (orphaned MCP servers, files, gitignore, brew, plugins) - Strip stale settings keys on re-sync to prevent Layer 3 re-injection of removed extraJSON/enabledPlugins - Remove stale template sections from CLAUDE file when pack updates drop sections - Clean up artifacts for newly excluded components via --customize - Refactor configure() into focused helper methods (resolveAllValues, preloadTemplates, installAndReconcileArtifacts, saveStateAndUpdateIndex)
bguidolim
force-pushed
the
feature/fix-customize-component-removal
branch
from
08d341c to
d6c6f8b
Compare
Doctor's globalScope() hardcoded excludedComponentIDs to empty set, causing excluded components to show as errors instead of skipped. Also fix --pack flag path which had the same issue.
This was referenced Mar 5, 2026
- Surface project state errors as warnings instead of silently swallowing
- Fix mangled docstring on resolveAllValues (had reconcileStaleArtifacts' doc prepended) - Add docstring to reconcileStaleArtifacts at its actual location - Remove unnecessary local aliases in DoctorRunner.resolveCheckScopes - Hoist duplicate ResourceRefCounter out of per-type branches - Add Constants.MCPScope.user/local and replace bare "user" strings
- reconcileStaleArtifacts: use inout to re-add failed removals for retry - reconcileTemplateSections: gate artifact record update on successful file write - Fix inaccurate docstrings (multi-phase, CLAUDE file, strategy-specific behavior) - Add comment explaining .shellCommand/.settingsMerge skip in exclusion handler - Add stale MCP server reconciliation test
- Add ClaudeCLI protocol and conform ClaudeIntegration to it; inject via ComponentExecutor and Configurator - Create MockClaudeCLI in tests that records method calls, inject into all makeConfigurator helpers - Update MCP tests to verify removal calls via mock instead of relying on real CLI side effects
- Make Configurator.claudeCLI non-optional with explicit init; resolve default at construction time - Remove `let claude = claudeCLI` aliases in ComponentExecutor, use property directly - Simplify test helpers to pass claudeCLI at init instead of two-step mutation
- Add warnings for failed removals in removeNewlyExcludedComponentArtifacts (MCP, file, brew, plugin) - Reconcile fileHashes in reconcileStaleArtifacts: remove on success, preserve previous hash on failure - Fix stale docstrings: "12-phase" → "multi-phase", ".mcs-project" → "scope's state file"
- Add `isAvailable` to ClaudeCLI protocol; ClaudeIntegration checks shell, MockClaudeCLI returns true - Replace `shell.commandExists` guards in ComponentExecutor with `claudeCLI.isAvailable` - Fixes CI failures where MCP tests failed because `claude` CLI is not installed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes multiple convergence gaps where the sync engine doesn't properly clean up stale artifacts. All gaps share a root pattern:
installArtifacts()rebuilds artifact records from scratch without diffing against previous records, so removed/renamed artifacts are silently dropped from tracking without cleanup.Stacked on #233 (template dependencies)
Changes
Fix 1: Stale artifact reconciliation (Gaps 1A, 2A)
reconcileStaleArtifacts()diffs previous vs currentPackArtifactRecordafterinstallArtifacts(), removing orphaned MCP servers, files, gitignore entries, brew packages, and pluginsFix 2: Stale settings key cleanup (Gaps 4A, 4B)
previousSettingsKeysparameter tocomposeSettingsprotocol methodSettings.removeKeys()before recomposing, passesdropKeystoSettings.save()to prevent Layer 3 re-injectiondropKeysto prevent Layer 3 re-injection of stale extraJSON keys from existing fileFix 3: Stale template section removal (Gap 5A)
reconcileTemplateSections()compares previously-tracked sections against what was written this runTemplateComposer.removeSection()Fix 0: Component exclusion cleanup
removeNewlyExcludedComponentArtifacts()handles--customizedeselection of individual componentsRefactoring
resolveAllValues(),preloadTemplates(),installAndReconcileArtifacts(),saveStateAndUpdateIndex()fromconfigure()for readabilityconfigure()reads as a clear table of contents of the 12-phase convergence flowTest plan
swift buildcompiles cleanlyswiftlint --strictandswiftformat --lintpass on all modified filesswift test— 650/654 pass (4 pre-existing PackFetcher failures from 1Password SSH agent)ConfiguratorExcludedComponentsTests(8 tests): MCP removal, file removal, first-run safety, re-inclusion, template dependency filteringStaleArtifactReconciliationTests(3 tests): stale file removal, stale settings cleanup, stale template section removalmcs sync→ verify old artifact is gonemcs sync --global --customize→ deselect → verify cleanup