fix(installer): stop --yes updates from silently deleting custom modules

[eugeniawang]

What

bmad install --action update --yes no longer deletes an installed module recorded in the manifest as source: custom whose source is still resolvable, and module removals are logged instead of happening silently.

Why

What we hit. Two separate BMAD installs, in two unrelated directories, each with its own _bmad/ — not nested, no shared install:

• Project A (<dev>/_bmad/) — core, bmm, bmb, cis, tea, bmad-loop, wds. All built-in or from the registry.
• Project B (<video-project>/_bmad/) — core, bmm, and manticore, the last installed from github.com/bmad-code-org/bmad-manticore and recorded source: custom.

One bmad install --directory <video-project> --action update --yes against Project B deleted <video-project>/_bmad/manticore/ outright, along with all 14 of its mc-* skills under that project's .claude/skills/ and .agents/skills/ — 310 files. It removed the module's manifest entry and exited 0 with nothing in the output about any of it. core and bmm in that same directory updated normally to 6.10.1-next.55. Only the custom module was lost, and the interactive path preserves it correctly.

Note the condition: a custom module whose source can no longer be found is preserved today, by the safety net below. It is the correctly-installed one, with its source still cached, that gets deleted.

getDefaultModules() (ui.js:1376-1407), used only by the --yes path, builds its keep-list from the built-in catalog and the external registry, so an installed custom module is never a candidate. _retainUnavailableInstalledModules() (ui.js:151-192) is the safety net, but it preserves a module only when no source can be found:

const customSource = await customMgr.findModuleSourceByCode(moduleId, { bmadDir });
if (!customSource) {
  preserveModules.push(moduleId);
}

A correctly-installed custom module has a findable source in ~/.bmad/cache/custom-modules/, so it is read as a deliberate deselection and _removeDeselectedModules() (installer.js:150-166) deletes it. The only log statement in that function is inside its catch, so a successful removal prints nothing.

The interactive path already has the guard, at ui.js:960-971:

// Identify installed modules that aren't official (previously installed
// community modules or custom-source modules). Preserve them on update;
// they can be managed via --custom-source, uninstall, or a dedicated installer.
const installedNonOfficial = [...installedModuleIds].filter((id) => !officialRegistryCodes.has(id));

And ui.js:345-350 shows the --yes path was assumed to behave the same way:

} else if (options.customSource && !options.yes) {
  // Custom source without --modules or --yes: start with empty list
  // (only custom source modules + core will be installed).
  // When --yes is also set, fall through to the --yes branch so all
  // installed modules are included alongside the custom source modules.

That comment assumes the --yes branch includes all installed modules. getDefaultModules does not include custom ones.

How

• getDefaultModules() — carry forward installed modules present in neither catalog, matching selectAllModules() and the assumption at ui.js:345-350. Ids are normalized through externalManager.resolveCanonicalCode() first, the same way _retainUnavailableInstalledModules() does, so a renamed or aliased id is not carried forward unresolved.
• _removeDeselectedModules() — log on the success path, so a module cannot disappear without a line of output.
• No change to --modules, which bypasses getDefaultModules() entirely (ui.js:338).

Testing

After the fix, manticore and its 14 skills survive the same command and core/bmm still update to 6.10.1-next.55. We also confirmed on the unpatched code that passing --custom-source https://github.com/bmad-code-org/bmad-manticore preserves the module, which pinned the cause to module selection rather than to removal.

Two limits on what we verified, both worth stating:

• We run this exact command daily against Project A, whose modules are all built-in or from the registry. 62 runs since early June, 55 of them clean (the other 7 failed on network or on an unrelated --tools requirement), and no module has ever been lost there. The failure appears only once a source: custom module is present.
• We ran one project with one custom module. That the same applies to any resolvable custom module follows from getDefaultModules() never consulting custom sources at all, but we have not run a multi-custom-module case.

[greptile-apps]

Greptile Summary

The PR updates non-interactive installer defaults so installed custom modules remain selected during updates and adds visibility when deselected modules are removed.

• Carries uncatalogued installed modules forward in getDefaultModules().
• Canonicalizes aliases and deduplicates resulting module selections.
• Logs successful module removals.
• Adds focused regression coverage and incorporates it into the test and quality scripts.

Confidence Score: 5/5

The PR appears safe to merge because no blocking failure remains in the eligible follow-up review scope.

No blocking failure remains.

Important Files Changed

Filename	Overview
tools/installer/ui.js	Extends non-interactive default selection to retain uncatalogued installed modules while canonicalizing and deduplicating module codes.
tools/installer/core/installer.js	Adds a warning after each deselected module directory is successfully removed.
test/test-installer-default-modules.js	Adds offline regression tests for custom-module preservation, alias canonicalization, deduplication, and channel decisions.
package.json	Registers the new regression test and includes it in the main test and quality workflows.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Read installed module IDs] --> B[Load official and registry catalogs]
    B --> C[Select catalog defaults and installed catalog modules]
    C --> D[Resolve each remaining installed ID]
    D --> E{Canonical ID already selected or core?}
    E -- Yes --> F[Skip duplicate]
    E -- No --> G[Carry module into update selection]
    F --> H[Run update]
    G --> H
    H --> I[Remove only genuinely deselected modules]
    I --> J[Log each removal]

Loading

_{Reviews (2): Last reviewed commit: "fix(installer): carry the canonical modu..." | Re-trigger Greptile}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 79dfc43f-6edc-4838-bc08-d1dad90b6954

📥 Commits

Reviewing files that changed from the base of the PR and between ade7a96 and 758aa56.

📒 Files selected for processing (2)

• tools/installer/core/installer.js
• tools/installer/ui.js

---

📝 Walkthrough

Walkthrough

The installer now preserves installed uncataloged modules during non-interactive updates and logs the identifier of each successfully removed deselected module.

Changes

Installer module lifecycle

Layer / File(s)	Summary
Preserve uncataloged modules tools/installer/ui.js	getDefaultModules resolves aliases and retains installed modules absent from built-in or external catalogs, except core.
Report removed modules tools/installer/core/installer.js	_removeDeselectedModules logs a warning after deleting a deselected module directory.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: bmadcode, alexeyv

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the primary fix: preserving custom modules during non-interactive installer updates.
Description check	✅ Passed	The description directly explains the custom-module deletion bug, the logging change, the implementation, and the testing performed.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[alexeyv]

Requesting one focused change before merge.

In getDefaultModules(), the new loop resolves an installed ID to its canonical code only for the membership checks, then appends the original ID:

const canonicalId = await externalManager.resolveCanonicalCode(moduleId);
// ...
defaultModules.push(moduleId);

For the existing bauto → bmad-loop alias, this still returns bauto. That contradicts the comment and PR description, and it bypasses code that indexes registry and channel state by the canonical code. A pinned or next installation can consequently fall back to the registry default during a full --yes update.

Please carry the canonical ID forward and deduplicate selections by canonical ID. Please also add focused deterministic regression tests covering:

• A resolvable custom module remains selected during --action update --yes.
• An installed alias such as bauto produces the canonical bmad-loop selection.
• The canonicalized selection retains the installed module's channel decision.

The custom-module preservation change and removal logging otherwise look good.

[eugeniawang]

Thanks — you were right on both counts. Pushed as 064c609.

Canonical ID carried forward. The loop now pushes canonicalId and dedupes on it:

for (const moduleId of installedModuleIds) {
  const canonicalId = await externalManager.resolveCanonicalCode(moduleId);
  if (canonicalId === 'core') continue;
  if (seen.has(canonicalId)) continue;
  seen.add(canonicalId);
  defaultModules.push(canonicalId);
}

One thing I had to change to make the dedupe actually canonical: seen was only populated by the built-in loop, so an installed registry module reached the carry-forward loop unrecorded. The old defaultModules.includes(...) checks papered over that. Both catalog loops now add to seen, and the carry-forward loop tests it alone — so bauto and bmad-loop installed side by side (which installer.js:1515 explicitly contemplates) yield exactly one bmad-loop entry.

Tests — test/test-installer-default-modules.js, registered as test:default-modules in the test and quality chains. Seven assertions across your three cases plus the dedupe pair. Both catalogs are stubbed at the prototype level and restored in a finally, so the tests are offline and independent of any BMAD install or ~/.bmad cache. The registry fixture declares bmad-loop with aliases: ['bauto'], so resolveCanonicalCode() runs for real rather than being stubbed.

Each test was confirmed to go red when the behaviour it guards is removed:

Removed	Assertion that failed
Carry-forward loop	custom module survives --action update --yes — expected "manticore", got ["bmm"]
Canonicalization (push(moduleId))	installed alias selects its canonical module code — expected "bmad-loop", got ["bauto"]
Canonicalization (push(moduleId))	canonicalized selection retains the installed --next channel decision — expected "next", got undefined
Dedupe by canonical ID	alias and canonical module deduplicate to one canonical selection — expected 1, got 3

That third one is your channel point made concrete: pre-fix the selection is ['bauto'], so buildPlan() — which keys on mod.code — has no bmad-loop entry at all and the --next decision is simply absent.

Full npm test is clean locally.