fix: address code review findings from PR #1590

- Filter 'core' from CLI --modules in update path for consistency
- Update selectAllModules() JSDoc to reflect core exclusion
- Fix ESM default-export unwrap to handle function/class exports

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Davor Racić 2026-02-08 10:27:25 +01:00
parent 6fd5db4aed
commit 3bdbb380a7
2 changed files with 8 additions and 4 deletions

View File

@ -1299,8 +1299,8 @@ class ModuleManager {
} else {
const { pathToFileURL } = require('node:url');
const imported = await import(pathToFileURL(installerPath).href);
// CJS module.exports lands on .default, ESM named exports are top-level
moduleInstaller = imported.default && typeof imported.default === 'object' ? imported.default : imported;
// CJS module.exports lands on .default; ESM default can be object, function, or class
moduleInstaller = imported.default == null ? imported : imported.default;
}
if (typeof moduleInstaller.install === 'function') {

View File

@ -361,6 +361,9 @@ class UI {
selectedModules.push(...customModuleResult.selectedCustomModules);
}
// Filter out core - it's always installed via installCore flag
selectedModules = selectedModules.filter((m) => m !== 'core');
// Get tool selection
const toolSelection = await this.promptToolSelection(confirmedDirectory, options);
@ -898,9 +901,10 @@ class UI {
}
/**
* Select all modules (core + official + community) using grouped multiselect
* Select all modules (official + community) using grouped multiselect.
* Core is shown as locked but filtered from the result since it's always installed separately.
* @param {Set} installedModuleIds - Currently installed module IDs
* @returns {Array} Selected module codes
* @returns {Array} Selected module codes (excluding core)
*/
async selectAllModules(installedModuleIds = new Set()) {
const { ModuleManager } = require('../installers/lib/modules/manager');