diff --git a/src/core-skills/bmad-module/scripts/cli.mjs b/src/core-skills/bmad-module/scripts/cli.mjs index eeac451ce..22f6fa42a 100644 --- a/src/core-skills/bmad-module/scripts/cli.mjs +++ b/src/core-skills/bmad-module/scripts/cli.mjs @@ -62,7 +62,13 @@ function parseArgs(argv) { export async function main() { const argv = process.argv.slice(2); - if (argv.length === 0 || argv[0] === '--help' || argv[0] === '-h') { + // An explicit help request is a successful invocation → exit 0. A bare call + // with no verb is an incomplete usage → EXIT.USAGE (2). + if (argv[0] === '--help' || argv[0] === '-h') { + printUsage(); + process.exit(0); + } + if (argv.length === 0) { printUsage(); process.exit(EXIT.USAGE); } diff --git a/src/core-skills/bmad-module/scripts/lib/config-gen.mjs b/src/core-skills/bmad-module/scripts/lib/config-gen.mjs index de52f3756..a32015ac3 100644 --- a/src/core-skills/bmad-module/scripts/lib/config-gen.mjs +++ b/src/core-skills/bmad-module/scripts/lib/config-gen.mjs @@ -39,19 +39,29 @@ export function formatTomlValue(value) { } // Minimal reverse of formatTomlValue for the scalars we read back (core values). -function parseTomlScalar(raw) { +// Exported for round-trip unit tests (test/test-bmad-module-source.mjs). +export function parseTomlScalar(raw) { const s = raw.trim(); if (s === 'true') return true; if (s === 'false') return false; if (/^-?\d+(\.\d+)?$/.test(s)) return Number(s); if (s.startsWith('"') && s.endsWith('"')) { - return s - .slice(1, -1) - .replaceAll('\\n', '\n') - .replaceAll('\\r', '\r') - .replaceAll('\\t', '\t') - .replaceAll('\\"', '"') - .replaceAll('\\\\', '\\'); + // Single left-to-right pass: each backslash consumes exactly one following + // char. A chained replaceAll would mis-handle round-tripped literal + // backslashes (e.g. `\\n` → `\` + newline instead of `\` + `n`), since an + // earlier pass can rewrite the escape introduced by a later one. The escape + // set here is the exact inverse of formatTomlValue (\\ \" \n \r \t). + const inner = s.slice(1, -1); + let out = ''; + for (let i = 0; i < inner.length; i++) { + if (inner[i] === '\\' && i + 1 < inner.length) { + const n = inner[++i]; + out += n === 'n' ? '\n' : n === 'r' ? '\r' : n === 't' ? '\t' : n === '"' ? '"' : n === '\\' ? '\\' : '\\' + n; + } else { + out += inner[i]; + } + } + return out; } return s; } diff --git a/src/core-skills/bmad-module/scripts/lib/fs-safe.mjs b/src/core-skills/bmad-module/scripts/lib/fs-safe.mjs index 2cd846d69..89a446020 100644 --- a/src/core-skills/bmad-module/scripts/lib/fs-safe.mjs +++ b/src/core-skills/bmad-module/scripts/lib/fs-safe.mjs @@ -126,7 +126,16 @@ export async function atomicSwapDir(stagedDir, targetDir) { if (hadTarget) await fsp.rm(backup, { recursive: true, force: true }); } catch (e) { await fsp.rm(sibling, { recursive: true, force: true }); - await fsp.rm(backup, { recursive: true, force: true }); + // Keep `backup` if it still exists — on a failed swap+rollback it is the + // only surviving copy of the previous install. Surface its location so the + // user can recover manually rather than silently destroying it here. + const backupSurvives = await fsp + .stat(backup) + .then(() => true) + .catch(() => false); + if (backupSurvives) { + process.stderr.write(`[bmad-module] previous install preserved at ${backup}\n`); + } throw e; } } diff --git a/src/core-skills/bmad-module/scripts/update.mjs b/src/core-skills/bmad-module/scripts/update.mjs index ebf42f251..484f20c3a 100644 --- a/src/core-skills/bmad-module/scripts/update.mjs +++ b/src/core-skills/bmad-module/scripts/update.mjs @@ -1,8 +1,10 @@ import path from 'node:path'; +import fsp from 'node:fs/promises'; import { EXIT, BmadModuleError } from './lib/exit.mjs'; import { findBmadDir } from './lib/bmad-dir.mjs'; import { parseSource, materializeSource } from './lib/source.mjs'; -import { readAndValidateManifest } from './lib/plugin-json.mjs'; +import { readAndValidateManifest, validateManifestObject, hasBmadPluginJson } from './lib/plugin-json.mjs'; +import { resolveLegacyModule } from './lib/legacy-resolver.mjs'; import { readUserIgnores, buildIgnoreMatcher, buildCopyPlan, rewriteManifestPaths, validateDeclaredPaths } from './lib/install-plan.mjs'; import { stageCopyPlan, atomicSwapDir, sha256File, pruneEmptyDirs, safePathInsideRoot } from './lib/fs-safe.mjs'; import { @@ -70,7 +72,29 @@ async function updateOne(bmadDir, projectDir, entry, opts) { return; } - const manifest = await readAndValidateManifest(materialized.dir); + // Re-read the manifest the same way install does: new-spec modules carry a + // `.claude-plugin/plugin.json#bmad`; legacy modules carry a + // `.claude-plugin/marketplace.json` resolved into a synthetic manifest. A + // legacy-installed module re-clones to the legacy format, so update must + // handle both — otherwise readAndValidateManifest throws BAD_MANIFEST and + // every legacy community module becomes un-updatable. + let manifest; + let synthesized = null; + if (await hasBmadPluginJson(materialized.dir)) { + manifest = await readAndValidateManifest(materialized.dir); + } else { + const legacy = await resolveLegacyModule(materialized.dir, { selector: code }); + if (!legacy) { + throw new BmadModuleError( + EXIT.BAD_MANIFEST, + `no .claude-plugin/plugin.json#bmad and no .claude-plugin/marketplace.json at ${materialized.dir}`, + ); + } + // Legacy first-party modules (gds, bmm, …) legitimately use reserved codes. + validateManifestObject(legacy.manifest, { allowReserved: true }); + manifest = legacy.manifest; + synthesized = legacy.synthesized; + } if (manifest.bmad.code !== code) { throw new BmadModuleError( EXIT.PREFIX_COLLISION, @@ -103,6 +127,18 @@ async function updateOne(bmadDir, projectDir, entry, opts) { ); } + // Strategy-5 legacy modules have no module.yaml/module-help.csv on disk — + // the resolver synthesized them. Write them into the throwaway temp source so + // buildCopyPlan/validateDeclaredPaths discover them via the normal path. + if (synthesized) { + if (synthesized['module.yaml']) { + await fsp.writeFile(path.join(materialized.dir, 'module.yaml'), synthesized['module.yaml'], 'utf8'); + } + if (synthesized['module-help.csv']) { + await fsp.writeFile(path.join(materialized.dir, 'module-help.csv'), synthesized['module-help.csv'], 'utf8'); + } + } + // Build new copy plan, stage, swap. validateDeclaredPaths(materialized.dir, manifest); const userIgnores = await readUserIgnores(materialized.dir, manifest); diff --git a/test/test-bmad-module-source.mjs b/test/test-bmad-module-source.mjs index 018f5c99e..94ac3992a 100644 --- a/test/test-bmad-module-source.mjs +++ b/test/test-bmad-module-source.mjs @@ -14,6 +14,7 @@ import { parseSource } from '../src/core-skills/bmad-module/scripts/lib/source.mjs'; import { valid, prerelease, compare, rcompare, validRange } from '../src/core-skills/bmad-module/scripts/lib/semver-lite.mjs'; import { parseGitHubRepo, normalizeStableTag } from '../src/core-skills/bmad-module/scripts/lib/channel-resolver.mjs'; +import { formatTomlValue, parseTomlScalar } from '../src/core-skills/bmad-module/scripts/lib/config-gen.mjs'; const colors = { reset: '', green: '', red: '', cyan: '', dim: '' }; let passed = 0; @@ -152,6 +153,18 @@ eq( 'normalizeStableTag excludes prereleases/invalid', ); +// ─── config-gen TOML scalar round-trip ──────────────────────────────────────── +// parseTomlScalar must be the exact inverse of formatTomlValue. Regression guard +// for the unescape bug where a literal backslash followed by `n` (e.g. a Windows +// path segment `\new`) was corrupted into a backslash + newline on read-back. +console.log(`\n${colors.cyan}config-gen TOML scalars${colors.reset}\n`); + +for (const v of [String.raw`a\new`, 'x\ty', 'l1\nl2', 'say "hi"', String.raw`back\\slash`, 'trailing\\', 'plain', '']) { + eq(parseTomlScalar(formatTomlValue(v)), v, `TOML round-trip: ${JSON.stringify(v)}`); +} +eq(parseTomlScalar('true'), true, 'parseTomlScalar bare true'); +eq(parseTomlScalar('42'), 42, 'parseTomlScalar bare number'); + // ─── Summary ────────────────────────────────────────────────────────────────── console.log(`\n${colors.cyan}Results: ${passed} passed, ${failed} failed${colors.reset}\n`); process.exit(failed > 0 ? 1 : 0);