Merge b161491ca8 into 1f99eb0496

* fix: preserve local custom module sources during quick update

Keep customModules in the generated main manifest so local custom
module source paths survive update runs. Load those preserved source
paths during stock quick update before falling back to the custom
cache directory.

This fixes the case where BMAD would drop customModules, lose the
original source path for a local module, and then skip the module or
try to re-cache from _bmad/_config/custom/<module>, which could fail
with ENOENT after the cache directory was removed.

Also adds an installation component regression test to verify
customModules and sourcePath are preserved in manifest generation.

Fixes #1582

* fix: ensure consistent formatting

* refactor: extract quick update custom source assembly

Move quick-update custom module source collection out of Installer and into
CustomModules as assembleQuickUpdateSources(). This keeps discoverPaths() focused on consuming prepared install inputs while
making the quick-update source assembly step explicit and easier to evolve.

Also:
- preserve customModules metadata in manifest regeneration for installed modules
- drop stale customModules entries when modules are no longer installed
- cover manifest preservation and manifest-backed quick-update sources in tests

docs/fr/how-to/non-interactive-installation.md

@@ -37,7 +37,19 @@ Nécessite [Node.js](https://nodejs.org) v20+ et `npx` (inclus avec npm).
 | `--user-name <nom>` | Nom à utiliser par les agents | Nom d'utilisateur système |
 | `--communication-language <langue>` | Langue de communication des agents | Anglais |
 | `--document-output-language <langue>` | Langue de sortie des documents | Anglais |
-| `--output-folder <chemin>` | Chemin du dossier de sortie | _bmad-output |
+| `--output-folder <chemin>` | Chemin du dossier de sortie (voir les règles de résolution ci-dessous) | `_bmad-output` |
+
+#### Résolution du chemin du dossier de sortie
+
+La valeur passée à `--output-folder` (ou saisie de manière interactive) est résolue selon ces règles :
+
+| Type d'entrée                 | Exemple                    | Résolu comme                                                 |
+|-------------------------------|----------------------------|--------------------------------------------------------------|
+| Chemin relatif (par défaut)   | `_bmad-output`             | `<racine-du-projet>/_bmad-output`                            |
+| Chemin relatif avec traversée | `../../shared-outputs`     | Chemin absolu normalisé — ex. `/Users/me/shared-outputs`     |
+| Chemin absolu                 | `/Users/me/shared-outputs` | Utilisé tel quel — la racine du projet n'est **pas** ajoutée |
+
+Le chemin résolu est ce que les agents et les workflows vont utiliser lors de l'écriture des fichiers de sortie. L'utilisation d'un chemin absolu ou d'un chemin relatif avec traversée vous permet de diriger tous les artefacts générés vers un répertoire en dehors de l'arborescence de votre projet — utile pour les configurations partagées ou les monorepos.
 
 ### Autres options
 
@@ -141,6 +153,7 @@ Les valeurs invalides entraîneront soit :
 
 :::tip[Bonnes pratiques]
 - Utilisez des chemins absolus pour `--directory` pour éviter toute ambiguïté
+- Utilisez un chemin absolu pour `--output-folder` lorsque vous souhaitez que les artefacts soient écrits en dehors de l'arborescence du projet (ex. un répertoire de sorties partagé dans un monorepo)
 - Testez les options localement avant de les utiliser dans des pipelines CI/CD
 - Combinez avec `-y` pour des installations vraiment sans surveillance
 - Utilisez `--debug` si vous rencontrez des problèmes lors de l'installation

src/bmm-skills/4-implementation/bmad-quick-dev/step-01-clarify-and-route.md

@@ -1,7 +1,7 @@
 ---
 wipFile: '{implementation_artifacts}/spec-wip.md'
 deferred_work_file: '{implementation_artifacts}/deferred-work.md'
-spec_file: '' # set at runtime for plan-code-review before leaving this step
+spec_file: '' # set at runtime for both routes before leaving this step
 ---
 
 # Step 1: Clarify and Route
@@ -52,11 +52,13 @@ Never ask extra questions if you already understand what the user intends.
    - On **K**: Proceed as-is.
 5. Route — choose exactly one:
 
+   Derive a valid kebab-case slug from the clarified intent. If the intent references a tracking identifier (story number, issue number, ticket ID), lead the slug with it (e.g. `3-2-digest-delivery`, `gh-47-fix-auth`). If `{implementation_artifacts}/spec-{slug}.md` already exists, append `-2`, `-3`, etc. Set `spec_file` = `{implementation_artifacts}/spec-{slug}.md`.
+
    **a) One-shot** — zero blast radius: no plausible path by which this change causes unintended consequences elsewhere. Clear intent, no architectural decisions.
+
    **EARLY EXIT** → `./step-oneshot.md`
 
    **b) Plan-code-review** — everything else. When uncertain whether blast radius is truly zero, choose this path.
-   1. Derive a valid kebab-case slug from the clarified intent. If the intent references a tracking identifier (story number, issue number, ticket ID), lead the slug with it (e.g. `3-2-digest-delivery`, `gh-47-fix-auth`). If `{implementation_artifacts}/spec-{slug}.md` already exists, append `-2`, `-3`, etc. Set `spec_file` = `{implementation_artifacts}/spec-{slug}.md`.
 
 
 ## NEXT

src/bmm-skills/4-implementation/bmad-quick-dev/step-oneshot.md

@@ -1,5 +1,6 @@
 ---
 deferred_work_file: '{implementation_artifacts}/deferred-work.md'
+spec_file: '' # set by step-01 before entering this step
 ---
 
 # Step One-Shot: Implement, Review, Present
@@ -29,19 +30,31 @@ Deduplicate all review findings. Three categories only:
 
 If a finding is caused by this change but too significant for a trivial patch, HALT and present it to the human for decision before proceeding.
 
+### Generate Spec Trace
+
+Set `{title}` = a concise title derived from the clarified intent.
+
+Write `{spec_file}` using `./spec-template.md`. Fill only these sections — delete all others:
+
+1. **Frontmatter** — set `title: '{title}'`, `type`, `created`, `status: 'done'`. Add `route: 'one-shot'`.
+2. **Title and Intent** — `# {title}` heading and `## Intent` with **Problem** and **Approach** lines. Reuse the summary you already generated for the terminal.
+3. **Suggested Review Order** — append after Intent. Build using the same convention as `./step-05-present.md` § "Generate Suggested Review Order" (spec-file-relative links, concern-based ordering, ultra-concise framing).
+
 ### Commit
 
 If version control is available and the tree is dirty, create a local commit with a conventional message derived from the intent. If VCS is unavailable, skip.
 
 ### Present
 
-1. Open all changed files in the user's editor so they can review the code directly:
-   - Resolve two sets of absolute paths: (1) the repository root (`git rev-parse --show-toplevel` — returns the worktree root when in a worktree, project root otherwise; if this fails, fall back to the current working directory), (2) each changed file. Run `code -r "{absolute-root}" <absolute-changed-file-paths>` — the root first so VS Code opens in the right context, then each changed file. Always double-quote paths to handle spaces and special characters.
-   - If `code` is not available (command fails), skip gracefully and list the file paths instead.
+1. Open the spec in the user's editor so they can click through the Suggested Review Order:
+   - Resolve two absolute paths: (1) the repository root (`git rev-parse --show-toplevel` — returns the worktree root when in a worktree, project root otherwise; if this fails, fall back to the current working directory), (2) `{spec_file}`. Run `code -r "{absolute-root}" "{absolute-spec-file}"` — the root first so VS Code opens in the right context, then the spec file. Always double-quote paths to handle spaces and special characters.
+   - If `code` is not available (command fails), skip gracefully and tell the user the spec file path instead.
 2. Display a summary in conversation output, including:
    - The commit hash (if one was created).
-   - List of files changed with one-line descriptions. Use CWD-relative paths with `:line` notation (e.g., `src/path/file.ts:42`) for terminal clickability. No leading `/`.
+   - List of files changed with one-line descriptions. Any file paths shown in conversation/terminal output must use CWD-relative format (no leading `/`) with `:line` notation (e.g., `src/path/file.ts:42`) for terminal clickability — this differs from spec-file links which use spec-file-relative paths.
    - Review findings breakdown: patches applied, items deferred, items rejected. If all findings were rejected, say so.
+   - A note that the spec is open in their editor (or the file path if it couldn't be opened). Mention that `{spec_file}` now contains a Suggested Review Order.
+   - **Navigation tip:** "Ctrl+click (Cmd+click on macOS) the links in the Suggested Review Order to jump to each stop."
 3. Offer to push and/or create a pull request.
 
 HALT and wait for human input.

test/test-installation-components.js

@@ -14,7 +14,9 @@
 const path = require('node:path');
 const os = require('node:os');
 const fs = require('fs-extra');
+const { Installer } = require('../tools/installer/core/installer');
 const { ManifestGenerator } = require('../tools/installer/core/manifest-generator');
+const { OfficialModules } = require('../tools/installer/modules/official-modules');
 const { IdeManager } = require('../tools/installer/ide/manager');
 const { clearCache, loadPlatformCodes } = require('../tools/installer/ide/platform-codes');
 
@@ -126,6 +128,56 @@ async function createSkillCollisionFixture() {
   return { root: fixtureRoot, bmadDir: fixtureDir };
 }
 
+async function createCustomModuleManifestFixture() {
+  const fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-custom-manifest-'));
+  const bmadDir = path.join(fixtureRoot, '_bmad');
+  const configDir = path.join(bmadDir, '_config');
+  const moduleSourceDir = path.join(fixtureRoot, 'test-module-source');
+  await fs.ensureDir(configDir);
+  await fs.ensureDir(moduleSourceDir);
+
+  const minimalAgent = '<agent name="Test" title="T"><persona>p</persona></agent>';
+  await fs.ensureDir(path.join(bmadDir, 'core', 'agents'));
+  await fs.writeFile(path.join(bmadDir, 'core', 'agents', 'test.md'), minimalAgent);
+  await fs.ensureDir(path.join(bmadDir, 'test-module', 'agents'));
+  await fs.writeFile(path.join(bmadDir, 'test-module', 'agents', 'test.md'), minimalAgent);
+  await fs.writeFile(path.join(moduleSourceDir, 'module.yaml'), ['code: test-module', 'name: Test Module', ''].join('\n'));
+
+  await fs.writeFile(
+    path.join(configDir, 'manifest.yaml'),
+    [
+      'installation:',
+      '  version: 6.2.2',
+      '  installDate: 2026-03-30T00:00:00.000Z',
+      '  lastUpdated: 2026-03-30T00:00:00.000Z',
+      'modules:',
+      '  - name: core',
+      '    version: 6.2.2',
+      '    installDate: 2026-03-30T00:00:00.000Z',
+      '    lastUpdated: 2026-03-30T00:00:00.000Z',
+      '    source: built-in',
+      '    npmPackage: null',
+      '    repoUrl: null',
+      '  - name: test-module',
+      '    version: null',
+      '    installDate: 2026-03-30T00:00:00.000Z',
+      '    lastUpdated: 2026-03-30T00:00:00.000Z',
+      '    source: custom',
+      '    npmPackage: null',
+      '    repoUrl: null',
+      'customModules:',
+      '  - id: test-module',
+      '    name: "Test Module"',
+      `    sourcePath: ${JSON.stringify(moduleSourceDir)}`,
+      'ides:',
+      '  - codex',
+      '',
+    ].join('\n'),
+  );
+
+  return { root: fixtureRoot, bmadDir, manifestPath: path.join(configDir, 'manifest.yaml'), moduleSourceDir };
+}
+
 /**
  * Test Suite
  */
@@ -1713,6 +1765,107 @@ async function runTests() {
 
   console.log('');
 
+  // ============================================================
+  // Suite 33: Main manifest preserves active customModules only
+  // ============================================================
+  console.log(`${colors.yellow}Test Suite 33: Preserve active customModules in main manifest${colors.reset}\n`);
+
+  let customManifestFixture = null;
+  try {
+    customManifestFixture = await createCustomModuleManifestFixture();
+    const yaml = require('yaml');
+    const originalManifest = yaml.parse(await fs.readFile(customManifestFixture.manifestPath, 'utf8'));
+    originalManifest.customModules.push({
+      id: 'removed-module',
+      name: 'Removed Module',
+      sourcePath: path.join(customManifestFixture.root, 'removed-module-source'),
+    });
+    await fs.writeFile(customManifestFixture.manifestPath, yaml.stringify(originalManifest), 'utf8');
+
+    const generator33 = new ManifestGenerator();
+    await generator33.generateManifests(customManifestFixture.bmadDir, ['core', 'test-module'], [], { ides: ['codex'] });
+
+    const updatedManifest = yaml.parse(await fs.readFile(customManifestFixture.manifestPath, 'utf8'));
+    const customModule = updatedManifest.customModules?.find((entry) => entry.id === 'test-module');
+
+    assert(Array.isArray(updatedManifest.customModules), 'Main manifest keeps customModules array');
+    assert(customModule !== undefined, 'Main manifest preserves existing custom module entry');
+    assert(
+      customModule && customModule.sourcePath === customManifestFixture.moduleSourceDir,
+      'Main manifest preserves custom module sourcePath',
+    );
+    assert(
+      !updatedManifest.customModules?.some((entry) => entry.id === 'removed-module'),
+      'Main manifest drops stale custom module entries',
+    );
+  } catch (error) {
+    assert(false, 'Main manifest preserves customModules test succeeds', error.message);
+  } finally {
+    if (customManifestFixture?.root) await fs.remove(customManifestFixture.root).catch(() => {});
+  }
+
+  console.log('');
+
+  // ============================================================
+  // Suite 34: Quick update uses manifest-backed custom sources
+  // ============================================================
+  console.log(`${colors.yellow}Test Suite 34: Quick update uses manifest-backed custom module sources${colors.reset}\n`);
+
+  let quickUpdateFixture = null;
+  const originalListAvailable34 = OfficialModules.prototype.listAvailable;
+  const originalLoadExistingConfig34 = OfficialModules.prototype.loadExistingConfig;
+  const originalCollectModuleConfigQuick34 = OfficialModules.prototype.collectModuleConfigQuick;
+  try {
+    quickUpdateFixture = await createCustomModuleManifestFixture();
+    const installer34 = new Installer();
+    installer34.externalModuleManager.hasModule = async () => false;
+    installer34.externalModuleManager.listAvailable = async () => [];
+
+    let capturedInstallConfig34 = null;
+    installer34.install = async (config) => {
+      capturedInstallConfig34 = config;
+      return { success: true };
+    };
+
+    OfficialModules.prototype.listAvailable = async function () {
+      return { modules: [], customModules: [] };
+    };
+    OfficialModules.prototype.loadExistingConfig = async function () {
+      this.collectedConfig = this.collectedConfig || {};
+    };
+    OfficialModules.prototype.collectModuleConfigQuick = async function (moduleName) {
+      this.collectedConfig = this.collectedConfig || {};
+      if (!this.collectedConfig[moduleName]) {
+        this.collectedConfig[moduleName] = {};
+      }
+      return false;
+    };
+
+    await installer34.quickUpdate({
+      directory: quickUpdateFixture.root,
+      skipPrompts: true,
+    });
+
+    const customModule34 = capturedInstallConfig34?._customModuleSources?.get('test-module');
+
+    assert(capturedInstallConfig34 !== null, 'Quick update forwards config to install');
+    assert(customModule34 !== undefined, 'Quick update keeps manifest-backed custom module updateable');
+    assert(customModule34 && customModule34.cached === false, 'Quick update uses manifest-backed source before cache');
+    assert(
+      customModule34 && customModule34.sourcePath === quickUpdateFixture.moduleSourceDir,
+      'Quick update uses preserved manifest sourcePath for custom modules',
+    );
+  } catch (error) {
+    assert(false, 'Quick update manifest-backed custom source test succeeds', error.message);
+  } finally {
+    OfficialModules.prototype.listAvailable = originalListAvailable34;
+    OfficialModules.prototype.loadExistingConfig = originalLoadExistingConfig34;
+    OfficialModules.prototype.collectModuleConfigQuick = originalCollectModuleConfigQuick34;
+    if (quickUpdateFixture?.root) await fs.remove(quickUpdateFixture.root).catch(() => {});
+  }
+
+  console.log('');
+
   // ============================================================
   // Summary
   // ============================================================

tools/installer/core/installer.js

@@ -1144,59 +1144,12 @@ class Installer {
     const configuredIdes = existingInstall.ides;
     const projectRoot = path.dirname(bmadDir);
 
-    // Get custom module sources: first from --custom-content (re-cache from source), then from cache
-    const customModuleSources = new Map();
-    if (config.customContent?.sources?.length > 0) {
-      for (const source of config.customContent.sources) {
-        if (source.id && source.path && (await fs.pathExists(source.path))) {
-          customModuleSources.set(source.id, {
-            id: source.id,
-            name: source.name || source.id,
-            sourcePath: source.path,
-            cached: false, // From CLI, will be re-cached
-          });
-        }
-      }
-    }
-    const cacheDir = path.join(bmadDir, '_config', 'custom');
-    if (await fs.pathExists(cacheDir)) {
-      const cachedModules = await fs.readdir(cacheDir, { withFileTypes: true });
-
-      for (const cachedModule of cachedModules) {
-        const moduleId = cachedModule.name;
-        const cachedPath = path.join(cacheDir, moduleId);
-
-        // Skip if path doesn't exist (broken symlink, deleted dir) - avoids lstat ENOENT
-        if (!(await fs.pathExists(cachedPath))) {
-          continue;
-        }
-        if (!cachedModule.isDirectory()) {
-          continue;
-        }
-
-        // Skip if we already have this module from manifest
-        if (customModuleSources.has(moduleId)) {
-          continue;
-        }
-
-        // Check if this is an external official module - skip cache for those
-        const isExternal = await this.externalModuleManager.hasModule(moduleId);
-        if (isExternal) {
-          continue;
-        }
-
-        // Check if this is actually a custom module (has module.yaml)
-        const moduleYamlPath = path.join(cachedPath, 'module.yaml');
-        if (await fs.pathExists(moduleYamlPath)) {
-          customModuleSources.set(moduleId, {
-            id: moduleId,
-            name: moduleId,
-            sourcePath: cachedPath,
-            cached: true,
-          });
-        }
-      }
-    }
+    const customModuleSources = await this.customModules.assembleQuickUpdateSources(
+      config,
+      existingInstall,
+      bmadDir,
+      this.externalModuleManager,
+    );
 
     // Get available modules (what we have source for)
     const availableModulesData = await new OfficialModules().listAvailable();

tools/installer/core/manifest-generator.js

@@ -377,10 +377,12 @@ class ManifestGenerator {
    */
   async writeMainManifest(cfgDir) {
     const manifestPath = path.join(cfgDir, 'manifest.yaml');
+    const installedModuleSet = new Set(this.modules);
 
     // Read existing manifest to preserve install date
     let existingInstallDate = null;
     const existingModulesMap = new Map();
+    let existingCustomModules = [];
 
     if (await fs.pathExists(manifestPath)) {
       try {
@@ -402,6 +404,12 @@ class ManifestGenerator {
             }
           }
         }
+
+        if (existingManifest.customModules && Array.isArray(existingManifest.customModules)) {
+          // We filter here so manifest regeneration preserves source metadata only for custom modules that
+          // are still installed. Without that, customModules can retain stale entries for modules that were removed.
+          existingCustomModules = existingManifest.customModules.filter((customModule) => installedModuleSet.has(customModule?.id));
+        }
       } catch {
         // If we can't read existing manifest, continue with defaults
       }
@@ -437,6 +445,7 @@ class ManifestGenerator {
         lastUpdated: new Date().toISOString(),
       },
       modules: updatedModules,
+      customModules: existingCustomModules,
       ides: this.selectedIdes,
     };
 

tools/installer/modules/custom-modules.js

@@ -192,6 +192,111 @@ class CustomModules {
 
     return this.paths;
   }
+
+  /**
+   * Assemble quick-update source candidates before install() hands them to discoverPaths().
+   * This exists because discoverPaths() consumes already-prepared quick-update sources,
+   * while quickUpdate() still has to build that source map from manifest, explicit inputs,
+   * and cache conventions.
+   * Precedence: manifest-backed paths, explicit sources override them, then cached modules.
+   * @param {Object} config - Quick update configuration
+   * @param {Object} existingInstall - Existing installation snapshot
+   * @param {string} bmadDir - BMAD directory
+   * @param {Object} externalModuleManager - External module manager
+   * @returns {Promise<Map<string, Object>>} Map of custom module ID to source info
+   */
+  async assembleQuickUpdateSources(config, existingInstall, bmadDir, externalModuleManager) {
+    const projectRoot = path.dirname(bmadDir);
+    const customModuleSources = new Map();
+
+    if (existingInstall.customModules) {
+      for (const customModule of existingInstall.customModules) {
+        // Skip if no ID - can't reliably track or re-cache without it
+        if (!customModule?.id) continue;
+
+        let sourcePath = customModule.sourcePath;
+        if (sourcePath && sourcePath.startsWith('_config')) {
+          // Paths are relative to BMAD dir, but we want absolute paths for install
+          sourcePath = path.join(bmadDir, sourcePath);
+        } else if (!sourcePath && customModule.relativePath) {
+          // Fall back to relativePath
+          sourcePath = path.resolve(projectRoot, customModule.relativePath);
+        } else if (sourcePath && !path.isAbsolute(sourcePath)) {
+          // If we have a sourcePath but it's not absolute, resolve it relative to project root
+          sourcePath = path.resolve(projectRoot, sourcePath);
+        }
+
+        // If we still don't have a valid source path, skip this module
+        if (!sourcePath || !(await fs.pathExists(sourcePath))) {
+          continue;
+        }
+
+        customModuleSources.set(customModule.id, {
+          id: customModule.id,
+          name: customModule.name || customModule.id,
+          sourcePath,
+          relativePath: customModule.relativePath,
+          cached: false,
+        });
+      }
+    }
+
+    if (config.customContent?.sources?.length > 0) {
+      for (const source of config.customContent.sources) {
+        if (source.id && source.path) {
+          customModuleSources.set(source.id, {
+            id: source.id,
+            name: source.name || source.id,
+            sourcePath: source.path,
+            cached: false, // From CLI, will be re-cached
+          });
+        }
+      }
+    }
+
+    const cacheDir = path.join(bmadDir, '_config', 'custom');
+    if (!(await fs.pathExists(cacheDir))) {
+      return customModuleSources;
+    }
+
+    const cachedModules = await fs.readdir(cacheDir, { withFileTypes: true });
+    for (const cachedModule of cachedModules) {
+      const moduleId = cachedModule.name;
+      const cachedPath = path.join(cacheDir, moduleId);
+
+      // Skip if path doesn't exist (broken symlink, deleted dir) - avoids lstat ENOENT
+      if (!(await fs.pathExists(cachedPath))) {
+        continue;
+      }
+      if (!cachedModule.isDirectory()) {
+        continue;
+      }
+
+      // Skip if we already have this module from manifest
+      if (customModuleSources.has(moduleId)) {
+        continue;
+      }
+
+      // Check if this is an external official module - skip cache for those
+      const isExternal = await externalModuleManager.hasModule(moduleId);
+      if (isExternal) {
+        continue;
+      }
+
+      // Check if this is actually a custom module (has module.yaml)
+      const moduleYamlPath = path.join(cachedPath, 'module.yaml');
+      if (await fs.pathExists(moduleYamlPath)) {
+        customModuleSources.set(moduleId, {
+          id: moduleId,
+          name: moduleId,
+          sourcePath: cachedPath,
+          cached: true,
+        });
+      }
+    }
+
+    return customModuleSources;
+  }
 }
 
 module.exports = { CustomModules };