fix(bmad-module): correctness — channel/.git, config-gen, install-plan, module resolution

- channel-resolver: strip a trailing slash before `.git` so `…/r.git/` resolves
  to repo `r` (was `r.git`, which broke stable-tag lookup).
- config-gen: drop orphaned [agents.*] blocks owned by this module (module=
  fallback) on regenerate, not only those still in the current module.yaml.
- install-plan: only honor file sources for the fixed-file Claude surfaces
  (hooks/mcpServers/lspServers/settings) so the rewritten manifest never points
  at an uncopied file; anchor customize.schemas rewrites on the owning skill dir
  so nested schema paths survive.
- cli: reject unknown flags with a usage error instead of running with defaults.
- project-root: prefer a resolution's exact moduleYamlPath over the first
  module.yaml found under the repo root (multi-module repos).
- official-modules: read the flattened _bmad/<code>/module.yaml first so new-spec
  modules honor their declared working directories.
- Add channel-resolver `.git/` regression cases.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
pbean 2026-06-20 15:58:13 -07:00
parent 08c73796a6
commit c845e78aab
8 changed files with 89 additions and 37 deletions

View File

@ -18,6 +18,8 @@ import { runList } from './list.mjs';
import { EXIT, BmadModuleError } from './lib/exit.mjs'; import { EXIT, BmadModuleError } from './lib/exit.mjs';
const VERBS = new Set(['install', 'update', 'remove', 'list']); const VERBS = new Set(['install', 'update', 'remove', 'list']);
const BOOLEAN_FLAGS = new Set(['dry-run', 'purge', 'all', 'json']);
const VALUE_FLAGS = new Set(['ref', 'channel', 'module', 'set', 'project-dir']);
function parseArgs(argv) { function parseArgs(argv) {
const out = { _: [], flags: {} }; const out = { _: [], flags: {} };
@ -26,8 +28,13 @@ function parseArgs(argv) {
const a = argv[i]; const a = argv[i];
if (a.startsWith('--')) { if (a.startsWith('--')) {
const key = a.slice(2); const key = a.slice(2);
// Reject unknown flags so typos fail fast instead of silently running
// with defaults.
if (!BOOLEAN_FLAGS.has(key) && !VALUE_FLAGS.has(key)) {
throw new BmadModuleError(EXIT.USAGE, `unknown flag --${key}`);
}
// boolean flags // boolean flags
if (['dry-run', 'purge', 'all', 'json'].includes(key)) { if (BOOLEAN_FLAGS.has(key)) {
out.flags[key] = true; out.flags[key] = true;
i++; i++;
continue; continue;

View File

@ -24,8 +24,8 @@ export function parseGitHubRepo(url) {
if (!url || typeof url !== 'string') return null; if (!url || typeof url !== 'string') return null;
const trimmed = url const trimmed = url
.trim() .trim()
.replace(/\.git$/, '') .replace(/\/+$/, '')
.replace(/\/$/, ''); .replace(/\.git$/, '');
const httpsMatch = trimmed.match(/^https?:\/\/github\.com\/([^/]+)\/([^/]+)(?:\/.*)?$/i); const httpsMatch = trimmed.match(/^https?:\/\/github\.com\/([^/]+)\/([^/]+)(?:\/.*)?$/i);
if (httpsMatch) return { owner: httpsMatch[1], repo: httpsMatch[2] }; if (httpsMatch) return { owner: httpsMatch[1], repo: httpsMatch[2] };
const sshMatch = trimmed.match(/^git@github\.com:([^/]+)\/([^/]+)$/i); const sshMatch = trimmed.match(/^git@github\.com:([^/]+)\/([^/]+)$/i);

View File

@ -249,10 +249,18 @@ export async function regenerateCentralConfig(bmadDir, code, opts = {}) {
{ {
const base = teamContent || TEAM_HEADER.join('\n') + '\n'; const base = teamContent || TEAM_HEADER.join('\n') + '\n';
const { preamble, blocks } = splitBlocks(base); const { preamble, blocks } = splitBlocks(base);
// Drop this module's prior [modules.<code>] and its [agents.*] (by code). // Drop this module's prior [modules.<code>] and its [agents.*] blocks. Match
const kept = blocks.filter( // current agent codes AND, as a fallback for removed/renamed agents (or a
(b) => b.header !== `modules.${sectionKey}` && !(b.header.startsWith('agents.') && agentCodes.has(b.header.slice('agents.'.length))), // missing module.yaml that leaves agentCodes empty), any [agents.*] block
); // whose `module = "<code>"` line marks it as owned by this module.
const kept = blocks.filter((b) => {
if (b.header === `modules.${sectionKey}` || b.header === `modules.${code}`) return false;
if (b.header.startsWith('agents.')) {
if (agentCodes.has(b.header.slice('agents.'.length))) return false;
if (b.lines.some((l) => /^module\s*=/.test(l) && parseTomlScalar(l.split('=').slice(1).join('=')) === code)) return false;
}
return true;
});
if (Object.keys(teamKv).length) kept.push(renderModuleBlock(sectionKey, teamKv)); if (Object.keys(teamKv).length) kept.push(renderModuleBlock(sectionKey, teamKv));
for (const a of agents) kept.push(renderAgentBlock(a)); for (const a of agents) kept.push(renderAgentBlock(a));
await fs.writeFile(teamPath, joinFile(preamble, kept), 'utf8'); await fs.writeFile(teamPath, joinFile(preamble, kept), 'utf8');

View File

@ -1,5 +1,6 @@
import { spawn } from 'node:child_process'; import { spawn } from 'node:child_process';
import { existsSync } from 'node:fs'; import { existsSync } from 'node:fs';
import path from 'node:path';
import { fileURLToPath } from 'node:url'; import { fileURLToPath } from 'node:url';
import { readManifestYaml } from './manifest-ops.mjs'; import { readManifestYaml } from './manifest-ops.mjs';
@ -20,6 +21,18 @@ import { readManifestYaml } from './manifest-ops.mjs';
// (the _bmad/ write already succeeded). // (the _bmad/ write already succeeded).
export async function distributeToIdes({ projectDir, bmadDir, prune = [] }) { export async function distributeToIdes({ projectDir, bmadDir, prune = [] }) {
const manifest = await readManifestYaml(bmadDir); const manifest = await readManifestYaml(bmadDir);
// readManifestYaml returns null for BOTH a missing manifest and a parse
// failure. A present-but-unreadable manifest is real config corruption — don't
// silently skip distribution; surface a repair hint. A genuinely absent
// manifest falls through to the "nothing configured" skip below.
if (manifest === null && existsSync(path.join(bmadDir, '_config', 'manifest.yaml'))) {
return {
ok: false,
hint:
'Could not read _bmad/_config/manifest.yaml (invalid YAML). Run `bmad install` to repair BMAD config, ' +
'then `bmad ide-sync` to push skills to your coding assistants.',
};
}
const ides = Array.isArray(manifest?.ides) ? manifest.ides.filter((i) => i && typeof i === 'string') : []; const ides = Array.isArray(manifest?.ides) ? manifest.ides.filter((i) => i && typeof i === 'string') : [];
if (ides.length === 0) { if (ides.length === 0) {
return { skipped: true }; return { skipped: true };

View File

@ -310,14 +310,14 @@ export async function buildCopyPlan(sourceDir, manifest, ignoreMatch) {
if (typeof v !== 'string') continue; if (typeof v !== 'string') continue;
const srcRel = stripDotSlash(v); const srcRel = stripDotSlash(v);
if (!srcRel) continue; if (!srcRel) continue;
// If the declared path is a directory, copy it under its basename. // These surfaces are single JSON files installed at a fixed canonical name
// (rewriteManifestPaths always points the manifest at `./hooks.json`,
// `./.mcp.json`, etc.). A directory source would be copied under its
// basename yet leave the manifest pointing at a file that was never written,
// so only file sources are honored here.
try { try {
const stat = await fs.stat(path.join(sourceDir, srcRel)); const stat = await fs.stat(path.join(sourceDir, srcRel));
if (stat.isDirectory()) { if (stat.isFile()) addFile(srcRel, destName);
await addDirRecursive(srcRel, path.posix.basename(srcRel));
} else if (stat.isFile()) {
addFile(srcRel, destName);
}
} catch { } catch {
/* missing — skip */ /* missing — skip */
} }
@ -363,21 +363,28 @@ export function rewriteManifestPaths(manifest) {
if (typeof out.bmad.moduleDefinition === 'string') out.bmad.moduleDefinition = './module.yaml'; if (typeof out.bmad.moduleDefinition === 'string') out.bmad.moduleDefinition = './module.yaml';
if (typeof out.bmad.moduleHelpCsv === 'string') out.bmad.moduleHelpCsv = './module-help.csv'; if (typeof out.bmad.moduleHelpCsv === 'string') out.bmad.moduleHelpCsv = './module-help.csv';
// customize.schemas — each entry lives inside its skill dir; the skill dir // customize.schemas — each entry lives inside a declared skill dir, which is
// itself is remapped to `skills/<basename>`, so the schema's new path is // remapped to `skills/<basename>`. Anchor on the owning skill dir and keep
// `./skills/<skill-basename>/<file>`. // every segment after it, so nested schemas (e.g. `<skill>/schemas/x.yaml`)
// land under the right skill instead of being collapsed to the last two
// segments.
const skillDirs = Array.isArray(manifest.skills)
? manifest.skills.filter((s) => typeof s === 'string').map((s) => stripDotSlash(s))
: [];
const schemas = out.bmad.customize?.schemas; const schemas = out.bmad.customize?.schemas;
if (Array.isArray(schemas)) { if (Array.isArray(schemas)) {
out.bmad.customize.schemas = schemas.map((entry) => { out.bmad.customize.schemas = schemas.map((entry) => {
if (typeof entry !== 'string') return entry; if (typeof entry !== 'string') return entry;
const srcRel = stripDotSlash(entry); const srcRel = stripDotSlash(entry);
const parts = srcRel.split('/'); const owner = skillDirs.find((sd) => sd && (srcRel === sd || srcRel.startsWith(sd + '/')));
// Heuristic: last two segments are `<skill-name>/<filename>`. if (owner) {
if (parts.length >= 2) { const remainder = srcRel.slice(owner.length + 1);
const file = parts.at(-1); return `./skills/${path.posix.basename(owner)}/${remainder}`;
const skill = parts.at(-2);
return `./skills/${skill}/${file}`;
} }
// Fallback when no declared skill owns the path: last two segments are
// assumed to be `<skill-name>/<filename>`.
const parts = srcRel.split('/');
if (parts.length >= 2) return `./skills/${parts.at(-2)}/${parts.at(-1)}`;
return `./${srcRel}`; return `./${srcRel}`;
}); });
} }

View File

@ -144,6 +144,8 @@ console.log(`\n${colors.cyan}channel-resolver${colors.reset}\n`);
eq(parseGitHubRepo('https://github.com/o/r/tree/main'), { owner: 'o', repo: 'r' }, 'parseGitHubRepo from deep URL'); eq(parseGitHubRepo('https://github.com/o/r/tree/main'), { owner: 'o', repo: 'r' }, 'parseGitHubRepo from deep URL');
eq(parseGitHubRepo('git@github.com:o/r'), { owner: 'o', repo: 'r' }, 'parseGitHubRepo from SSH'); eq(parseGitHubRepo('git@github.com:o/r'), { owner: 'o', repo: 'r' }, 'parseGitHubRepo from SSH');
eq(parseGitHubRepo('https://gitlab.com/o/r'), null, 'parseGitHubRepo null for non-GitHub'); eq(parseGitHubRepo('https://gitlab.com/o/r'), null, 'parseGitHubRepo null for non-GitHub');
eq(parseGitHubRepo('https://github.com/o/r.git'), { owner: 'o', repo: 'r' }, 'parseGitHubRepo strips .git');
eq(parseGitHubRepo('https://github.com/o/r.git/'), { owner: 'o', repo: 'r' }, 'parseGitHubRepo strips .git before trailing slash');
eq( eq(
[normalizeStableTag('v1.7.0'), normalizeStableTag('1.0.0-rc.1'), normalizeStableTag('nope')], [normalizeStableTag('v1.7.0'), normalizeStableTag('1.0.0-rc.1'), normalizeStableTag('nope')],
['1.7.0', null, null], ['1.7.0', null, null],

View File

@ -694,6 +694,13 @@ class OfficialModules {
const projectRoot = path.dirname(bmadDir); const projectRoot = path.dirname(bmadDir);
const emptyResult = { createdDirs: [], movedDirs: [], createdWdsFolders: [] }; const emptyResult = { createdDirs: [], movedDirs: [], createdWdsFolders: [] };
// Prefer the flattened installed module.yaml. buildCopyPlan() copies a
// new-spec module's moduleDefinition to _bmad/<code>/module.yaml, where it
// carries the canonical `directories` declarations — but its source tree may
// keep module.yaml under a skill asset path that findModuleSource() can't
// locate, which would otherwise skip the declared working dirs.
let moduleYamlPath = path.join(bmadDir, moduleName, 'module.yaml');
if (!(await fs.pathExists(moduleYamlPath))) {
// Special handling for core module - it's in src/core-skills not src/modules // Special handling for core module - it's in src/core-skills not src/modules
let sourcePath; let sourcePath;
if (moduleName === 'core') { if (moduleName === 'core') {
@ -706,10 +713,11 @@ class OfficialModules {
} }
// Read module.yaml to find the `directories` key // Read module.yaml to find the `directories` key
const moduleYamlPath = path.join(sourcePath, 'module.yaml'); moduleYamlPath = path.join(sourcePath, 'module.yaml');
if (!(await fs.pathExists(moduleYamlPath))) { if (!(await fs.pathExists(moduleYamlPath))) {
return emptyResult; // No module.yaml, skip return emptyResult; // No module.yaml, skip
} }
}
let moduleYaml; let moduleYaml;
try { try {

View File

@ -176,7 +176,14 @@ async function resolveInstalledModuleYaml(moduleName) {
// from its marketplace plugin name (e.g. bmad-creative-intelligence-suite) // from its marketplace plugin name (e.g. bmad-creative-intelligence-suite)
// can be tracked downstream under any of the three — match all of them. // can be tracked downstream under any of the three — match all of them.
const matches = mod.code === moduleName || mod.name === moduleName || mod.pluginName === moduleName; const matches = mod.code === moduleName || mod.name === moduleName || mod.pluginName === moduleName;
if (matches && mod.localPath) { if (!matches) continue;
// Prefer the resolution's exact module.yaml — searchRoot(localPath) returns
// the FIRST module.yaml under the root, which can be the wrong one in a
// multi-module/multi-plugin repo resolved by pluginName.
if (mod.moduleYamlPath && (await fs.pathExists(mod.moduleYamlPath))) {
return mod.moduleYamlPath;
}
if (mod.localPath) {
const found = await searchRoot(mod.localPath); const found = await searchRoot(mod.localPath);
if (found) return found; if (found) return found;
} }