fix(installer): address automator review feedback

Author: bma-d

docs/how-to/install-bmad.md

@@ -38,6 +38,10 @@ The interactive flow asks you five things:
 
 Accept the defaults and you land on the latest stable release of every module, configured for your chosen tool.
 
+:::caution[BMad Automator constraints]
+`bma` installs runnable Automator skills only for the Claude Code entrypoint. Codex is supported as a worker target only, and worker sessions currently require `tmux` on macOS.
+:::
+
 :::tip[Just want the newest prerelease?]
 
 ```bash

test/test-installation-components.js

@@ -116,6 +116,7 @@ async function createAutomatorBmadFixture() {
       path.join(skillDir, 'SKILL.md'),
       ['---', `name: ${skillName}`, 'description: Automator skill', '---', '', 'Automator body.'].join('\n'),
     );
+    await fs.writeFile(path.join(skillDir, 'workflow.md'), `# ${skillName}\n\nAutomator workflow body.\n`);
   }
 
   return fixtureDir;
@@ -2826,6 +2827,28 @@ async function runTests() {
       automatorInfo42.installTargets.length === 1 && automatorInfo42.installTargets.includes('claude-code'),
       'BMad Automator is limited to Claude Code skill installation',
     );
+    const normalizedInfo42 = externalManager42._normalizeModule({
+      name: 'bad-shapes',
+      code: 'bad',
+      repository: 'https://example.com/bad.git',
+      install_targets: 'claude-code',
+      worker_targets: { bad: true },
+      requirements: ['tmux', { bad: true }, false],
+    });
+    assert(
+      Array.isArray(normalizedInfo42.installTargets) && normalizedInfo42.installTargets.includes('claude-code'),
+      'External module install targets normalize scalar values to arrays',
+    );
+    assert(
+      Array.isArray(normalizedInfo42.workerTargets) && normalizedInfo42.workerTargets.length === 0,
+      'External module worker targets drop invalid shapes',
+    );
+    assert(
+      normalizedInfo42.requirements.length === 2 &&
+        normalizedInfo42.requirements.includes('tmux') &&
+        normalizedInfo42.requirements.includes('false'),
+      'External module requirements normalize scalar array entries',
+    );
 
     tempProjectDir42 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-automator-target-'));
     installedBmadDir42 = await createAutomatorBmadFixture();
@@ -2846,6 +2869,30 @@ async function runTests() {
       !(await fs.pathExists(path.join(tempProjectDir42, '.agents', 'skills', 'bmad-story-automator', 'SKILL.md'))),
       'Codex setup skips Claude Code-only automator skill',
     );
+    assert(
+      !(await fs.pathExists(path.join(tempProjectDir42, '.agents', 'skills', 'bmad-story-automator-review', 'SKILL.md'))),
+      'Codex setup skips Claude Code-only automator review skill',
+    );
+
+    const escapeRoot42 = await fs.mkdtemp(path.join(os.tmpdir(), 'bmad-source-root-'));
+    const escapeRepo42 = path.join(escapeRoot42, 'repo');
+    await fs.ensureDir(escapeRepo42);
+    const escapeManager42 = new ExternalModuleManager();
+    escapeManager42.getModuleByCode = async () => ({
+      code: 'escape',
+      builtIn: false,
+      sourceRoot: '../outside',
+    });
+    escapeManager42.cloneExternalModule = async () => escapeRepo42;
+    let rejectedEscapingSourceRoot42 = false;
+    try {
+      await escapeManager42.findExternalModuleSource('escape');
+    } catch (error) {
+      rejectedEscapingSourceRoot42 = error.message.includes('source-root escapes repository');
+    } finally {
+      await fs.remove(escapeRoot42).catch(() => {});
+    }
+    assert(rejectedEscapingSourceRoot42, 'External module source-root cannot escape cloned repository');
 
     const claudeResult42 = await ideManager42.setup('claude-code', tempProjectDir42, installedBmadDir42, {
       silent: true,
@@ -2860,8 +2907,12 @@ async function runTests() {
       await fs.pathExists(path.join(tempProjectDir42, '.claude', 'skills', 'bmad-story-automator-review', 'SKILL.md')),
       'Claude Code setup installs automator review skill',
     );
+    assert(
+      await fs.pathExists(path.join(tempProjectDir42, '.claude', 'skills', 'bmad-story-automator-review', 'workflow.md')),
+      'Claude Code setup copies automator workflow files',
+    );
   } catch (error) {
-    assert(false, 'Automator external skill-only module test succeeds', error.message);
+    assert(false, `Automator external skill-only module test succeeds: ${error.message}`);
   } finally {
     if (tempProjectDir42) await fs.remove(tempProjectDir42).catch(() => {});
     if (installedBmadDir42) await fs.remove(path.dirname(installedBmadDir42)).catch(() => {});

tools/installer/core/manifest-generator.js

@@ -810,8 +810,22 @@ class ManifestGenerator {
     const modulePath = path.join(this.bmadDir, moduleName);
     if (!(await fs.pathExists(modulePath))) return false;
     if (await fs.pathExists(path.join(modulePath, 'module.yaml'))) return false;
+    if (!(await this._moduleUsesSourceRoot(moduleName))) return false;
     return this._hasSkillMdRecursive(modulePath);
   }
+
+  async _moduleUsesSourceRoot(moduleName) {
+    if (!this.sourceRootModuleCodes) {
+      try {
+        const { ExternalModuleManager } = require('../modules/external-manager');
+        const externalModules = await new ExternalModuleManager().listAvailable();
+        this.sourceRootModuleCodes = new Set(externalModules.filter((mod) => mod.sourceRoot).map((mod) => mod.code));
+      } catch {
+        this.sourceRootModuleCodes = new Set();
+      }
+    }
+    return this.sourceRootModuleCodes.has(moduleName);
+  }
 }
 
 /**

tools/installer/ide/_config-driven.js

@@ -218,7 +218,7 @@ class ConfigDrivenIdeSetup {
     const { ExternalModuleManager } = require('../modules/external-manager');
     this.externalModuleManager = this.externalModuleManager || new ExternalModuleManager();
     const moduleInfo = await this.externalModuleManager.getModuleByCode(moduleName);
-    const targets = moduleInfo?.installTargets || null;
+    const targets = moduleInfo?.installTargets?.length ? moduleInfo.installTargets : null;
     this.moduleTargetCache.set(moduleName, targets);
 
     return !targets || targets.includes(this.name);

tools/installer/modules/external-manager.js

@@ -16,6 +16,15 @@ function normalizeChannelName(raw) {
   return VALID_CHANNELS.has(lower) ? lower : null;
 }
 
+function normalizeStringList(raw) {
+  if (raw == null || raw === '') return [];
+  const values = Array.isArray(raw) ? raw : [raw];
+  return values
+    .filter((value) => ['string', 'number', 'boolean'].includes(typeof value))
+    .map((value) => String(value).trim())
+    .filter(Boolean);
+}
+
 /**
  * Conservative quoting for tag names passed to git commands. Tags are
  * user-typed (--pin) or come from the GitHub API. Only allow the semver
@@ -120,6 +129,9 @@ class ExternalModuleManager {
    * @returns {Object} Normalized module info
    */
   _normalizeModule(mod, key) {
+    const installTargets = mod.install_targets ?? mod['install-targets'] ?? mod.installTargets;
+    const workerTargets = mod.worker_targets ?? mod['worker-targets'] ?? mod.workerTargets;
+
     return {
       key: key || mod.name,
       url: mod.repository || mod.url,
@@ -131,9 +143,9 @@ class ExternalModuleManager {
       defaultSelected: mod.default_selected === true || mod.defaultSelected === true,
       type: mod.type || 'bmad-org',
       npmPackage: mod.npm_package || mod.npmPackage || null,
-      installTargets: mod.install_targets || mod['install-targets'] || mod.installTargets || null,
-      workerTargets: mod.worker_targets || mod['worker-targets'] || mod.workerTargets || [],
-      requirements: mod.requirements || [],
+      installTargets: normalizeStringList(installTargets),
+      workerTargets: normalizeStringList(workerTargets),
+      requirements: normalizeStringList(mod.requirements),
       installNote: mod.install_note || mod['install-note'] || mod.installNote || null,
       defaultChannel: normalizeChannelName(mod.default_channel || mod.defaultChannel) || 'stable',
       builtIn: mod.built_in === true,
@@ -513,7 +525,12 @@ class ExternalModuleManager {
     const cloneDir = await this.cloneExternalModule(moduleCode, options);
 
     if (moduleInfo.sourceRoot) {
-      const sourceRoot = path.join(cloneDir, moduleInfo.sourceRoot);
+      const repoRoot = path.resolve(cloneDir);
+      const sourceRoot = path.resolve(repoRoot, moduleInfo.sourceRoot);
+      const relativeSourceRoot = path.relative(repoRoot, sourceRoot);
+      if (relativeSourceRoot === '..' || relativeSourceRoot.startsWith(`..${path.sep}`) || path.isAbsolute(relativeSourceRoot)) {
+        throw new Error(`External module '${moduleCode}' source-root escapes repository: ${moduleInfo.sourceRoot}`);
+      }
       if (!(await fs.pathExists(sourceRoot))) {
         throw new Error(`External module '${moduleCode}' source-root not found: ${moduleInfo.sourceRoot}`);
       }