fix: address code review findings from PR #1612
- Restore saved IDE configurations when removal is cancelled or skipped in non-interactive mode, preventing silent config downgrade (e.g., Codex 'project' install reverting to 'global') - Short-circuit IDE removal block when skipPrompts is true to eliminate unnecessary warning/cancelled log output in --yes mode - Add null guard on config.ides before pushing re-added IDEs - Return empty result object from createModuleDirectories early exits instead of undefined for a uniform return contract Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
f967fdde89
commit
ef0c911722
|
|
@ -710,6 +710,17 @@ class Installer {
|
||||||
const idesToRemove = [...previouslyInstalledIdes].filter((ide) => !newlySelectedIdes.has(ide));
|
const idesToRemove = [...previouslyInstalledIdes].filter((ide) => !newlySelectedIdes.has(ide));
|
||||||
|
|
||||||
if (idesToRemove.length > 0) {
|
if (idesToRemove.length > 0) {
|
||||||
|
if (config.skipPrompts) {
|
||||||
|
// Non-interactive mode: silently preserve existing IDE configs
|
||||||
|
if (!config.ides) config.ides = [];
|
||||||
|
const savedIdeConfigs = await this.ideConfigManager.loadAllIdeConfigs(bmadDir);
|
||||||
|
for (const ide of idesToRemove) {
|
||||||
|
config.ides.push(ide);
|
||||||
|
if (savedIdeConfigs[ide] && !ideConfigurations[ide]) {
|
||||||
|
ideConfigurations[ide] = savedIdeConfigs[ide];
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} else {
|
||||||
if (spinner.isSpinning) {
|
if (spinner.isSpinning) {
|
||||||
spinner.stop('Reviewing IDE changes');
|
spinner.stop('Reviewing IDE changes');
|
||||||
}
|
}
|
||||||
|
|
@ -719,10 +730,7 @@ class Installer {
|
||||||
await prompts.log.error(` - ${ide}`);
|
await prompts.log.error(` - ${ide}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
// In non-interactive mode, preserve existing configs (matches prompt default: false)
|
const confirmRemoval = await prompts.confirm({
|
||||||
const confirmRemoval = config.skipPrompts
|
|
||||||
? false
|
|
||||||
: await prompts.confirm({
|
|
||||||
message: `Remove BMAD configuration for ${idesToRemove.length} IDE(s)?`,
|
message: `Remove BMAD configuration for ${idesToRemove.length} IDE(s)?`,
|
||||||
default: false,
|
default: false,
|
||||||
});
|
});
|
||||||
|
|
@ -744,15 +752,21 @@ class Installer {
|
||||||
await prompts.log.success(` Removed ${idesToRemove.length} IDE(s)`);
|
await prompts.log.success(` Removed ${idesToRemove.length} IDE(s)`);
|
||||||
} else {
|
} else {
|
||||||
await prompts.log.message(' IDE removal cancelled');
|
await prompts.log.message(' IDE removal cancelled');
|
||||||
// Add IDEs back to selection since user cancelled removal
|
// Add IDEs back to selection and restore their saved configurations
|
||||||
|
if (!config.ides) config.ides = [];
|
||||||
|
const savedIdeConfigs = await this.ideConfigManager.loadAllIdeConfigs(bmadDir);
|
||||||
for (const ide of idesToRemove) {
|
for (const ide of idesToRemove) {
|
||||||
config.ides.push(ide);
|
config.ides.push(ide);
|
||||||
|
if (savedIdeConfigs[ide] && !ideConfigurations[ide]) {
|
||||||
|
ideConfigurations[ide] = savedIdeConfigs[ide];
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
spinner.start('Preparing installation...');
|
spinner.start('Preparing installation...');
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Results collector for consolidated summary
|
// Results collector for consolidated summary
|
||||||
const results = [];
|
const results = [];
|
||||||
|
|
|
||||||
|
|
@ -1252,11 +1252,12 @@ class ModuleManager {
|
||||||
* @param {Object} options - Installation options
|
* @param {Object} options - Installation options
|
||||||
* @param {Object} options.moduleConfig - Module configuration from config collector
|
* @param {Object} options.moduleConfig - Module configuration from config collector
|
||||||
* @param {Object} options.coreConfig - Core configuration
|
* @param {Object} options.coreConfig - Core configuration
|
||||||
* @returns {Promise<{createdDirs: string[], createdWdsFolders: string[]} | undefined>} Created directories info
|
* @returns {Promise<{createdDirs: string[], createdWdsFolders: string[]}>} Created directories info
|
||||||
*/
|
*/
|
||||||
async createModuleDirectories(moduleName, bmadDir, options = {}) {
|
async createModuleDirectories(moduleName, bmadDir, options = {}) {
|
||||||
const moduleConfig = options.moduleConfig || {};
|
const moduleConfig = options.moduleConfig || {};
|
||||||
const projectRoot = path.dirname(bmadDir);
|
const projectRoot = path.dirname(bmadDir);
|
||||||
|
const emptyResult = { createdDirs: [], createdWdsFolders: [] };
|
||||||
|
|
||||||
// Special handling for core module - it's in src/core not src/modules
|
// Special handling for core module - it's in src/core not src/modules
|
||||||
let sourcePath;
|
let sourcePath;
|
||||||
|
|
@ -1265,14 +1266,14 @@ class ModuleManager {
|
||||||
} else {
|
} else {
|
||||||
sourcePath = await this.findModuleSource(moduleName, { silent: true });
|
sourcePath = await this.findModuleSource(moduleName, { silent: true });
|
||||||
if (!sourcePath) {
|
if (!sourcePath) {
|
||||||
return; // No source found, skip
|
return emptyResult; // No source found, skip
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Read module.yaml to find the `directories` key
|
// Read module.yaml to find the `directories` key
|
||||||
const moduleYamlPath = path.join(sourcePath, 'module.yaml');
|
const moduleYamlPath = path.join(sourcePath, 'module.yaml');
|
||||||
if (!(await fs.pathExists(moduleYamlPath))) {
|
if (!(await fs.pathExists(moduleYamlPath))) {
|
||||||
return; // No module.yaml, skip
|
return emptyResult; // No module.yaml, skip
|
||||||
}
|
}
|
||||||
|
|
||||||
let moduleYaml;
|
let moduleYaml;
|
||||||
|
|
@ -1280,11 +1281,11 @@ class ModuleManager {
|
||||||
const yamlContent = await fs.readFile(moduleYamlPath, 'utf8');
|
const yamlContent = await fs.readFile(moduleYamlPath, 'utf8');
|
||||||
moduleYaml = yaml.parse(yamlContent);
|
moduleYaml = yaml.parse(yamlContent);
|
||||||
} catch {
|
} catch {
|
||||||
return; // Invalid YAML, skip
|
return emptyResult; // Invalid YAML, skip
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!moduleYaml || !moduleYaml.directories) {
|
if (!moduleYaml || !moduleYaml.directories) {
|
||||||
return; // No directories declared, skip
|
return emptyResult; // No directories declared, skip
|
||||||
}
|
}
|
||||||
|
|
||||||
const directories = moduleYaml.directories;
|
const directories = moduleYaml.directories;
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue