fix(installer): preserve stale installed modules during update#2391
Conversation
|
Addressed on the latest branch tip in The retained-module config fix now preserves prior Scope is still intentionally narrow:
Validation: |
|
Cross-linking for review context:
Recommended merge order:
|
76bf87c to
9cf7ba7
Compare
9cf7ba7 to
3cdee79
Compare
|
@coderabbitai review |
🤖 Augment PR SummarySummary: Improves update/quick-update safety for installs that contain modules whose source can no longer be resolved (e.g., legacy/stale modules), preventing them from being removed or having their config/skills silently dropped. Changes:
Technical Notes: The installer now threads preserved-module context into both module removal and manifest/IDE cleanup so legacy modules stay installed and keep their recorded skill/config metadata during updates. 🤖 Was this summary useful? React with 👍 or 👎 |
| const preservedModuleSet = new Set(preservedModules || []); | ||
| const ids = new Set(); | ||
| for (const row of previousRows || []) { | ||
| if (row.canonicalId && !preservedModuleSet.has(row.module)) { |
There was a problem hiding this comment.
In _getPreviousSkillIdsForCleanup, preservation depends on row.module being present in the previous skill-manifest.csv; if older manifests are missing the module column (or it’s blank), preserved-module skills could still be scheduled for cleanup. Consider a fallback derivation (e.g., from row.path) before comparing against preservedModules to make the stale-module compatibility path more robust.
Severity: medium
Other Locations
tools/installer/core/installer.js:455
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR adds module preservation support to the installer's install/update flows. UI detects unselected installed modules lacking custom sources and marks them for preservation. The installer reads existing skill-manifest.csv before overwrite, excludes preserved modules from removal, tracks their files for manifest purposes, and re-appends their rows post-generation with deduping. IDE cleanup logic adjusts flow detection based on skill-ID truthiness. ChangesModule Preservation During Install and Update
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
tools/installer/ide/_config-driven.jsOops! Something went wrong! :( ESLint: 9.39.4 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by tools/installer/core/installer.jsOops! Something went wrong! :( ESLint: 9.39.4 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by tools/installer/ui.jsOops! Something went wrong! :( ESLint: 9.39.4 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Documents the fix landed in #2391 and the manual step required for users who installed the experimental BMad Automator module under its previous `baut` code.
Summary
Fixes the update-time compatibility path for already-installed stale modules such as legacy Automator
bautentries.Changes
Why
This PR is about safe handling of existing installs. It prevents update flows from breaking or silently losing config when a previously installed module can no longer be resolved from current sources.
Scope limits
Relationship To Other PRs
This PR is independent.
It solves the stale-install compatibility problem only. The separate “make Automator installable again” work is handled by:
BMAD-METHOD#2392installer support forsource_rootbmad-plugins-marketplace#26registry metadata for AutomatorValidation
node test/test-installation-components.js