From 08dc71f8ccfa67075a410cdcbc8a4219ec0069cc Mon Sep 17 00:00:00 2001 From: pbean Date: Sat, 20 Jun 2026 16:00:09 -0700 Subject: [PATCH] chore(bmad-module): frontmatter/CRLF, test hygiene, fixture corrections - frontmatter: accept a block that ends at EOF (optional trailing newline). - legacy-resolver: accept CRLF frontmatter delimiters. - integration.test.sh: unique mktemp stderr capture, explicit if/then/else assertions (drop the SC2015 && ok || ko chains), plus unknown-flag and invalid-channel usage-error cases. - test-installation-components: clear the resolution cache in a finally. - acme-devlog fixtures: correct the uninstall note to the flat TOML layout, replace `ls -t | head` with a glob/-nt lookup, drop the always-on devlog config file: fact, "run" -> "invoke" wording, and reconcile the devlog-write template contract with the bundled template. Co-Authored-By: Claude Opus 4.8 --- .../bmad-module/scripts/lib/frontmatter.mjs | 2 +- .../scripts/lib/legacy-resolver.mjs | 2 +- .../comprehensive/acme-devlog/README.md | 2 +- .../acme-devlog/scripts/fetch-git-history.sh | 12 ++- .../bmad-agent-historian/customize.toml | 5 +- .../skills/bmad-devlog-summarize/SKILL.md | 2 +- .../skills/bmad-devlog-write/SKILL.md | 4 +- .../bmad-module/tests/integration.test.sh | 88 +++++++++++-------- test/test-installation-components.js | 8 +- 9 files changed, 78 insertions(+), 47 deletions(-) diff --git a/src/core-skills/bmad-module/scripts/lib/frontmatter.mjs b/src/core-skills/bmad-module/scripts/lib/frontmatter.mjs index 7982ceb49..e3bb41800 100644 --- a/src/core-skills/bmad-module/scripts/lib/frontmatter.mjs +++ b/src/core-skills/bmad-module/scripts/lib/frontmatter.mjs @@ -3,7 +3,7 @@ import { parse as parseYaml } from './vendor/yaml.mjs'; // Parse YAML frontmatter from a markdown string. Returns the parsed object, // or null if no frontmatter block is present / it failed to parse. export function parseFrontmatter(content) { - const m = content.match(/^---\r?\n([\s\S]*?)\r?\n---\r?\n/); + const m = content.match(/^---\r?\n([\s\S]*?)\r?\n---(?:\r?\n|$)/); if (!m) return null; try { return parseYaml(m[1]); diff --git a/src/core-skills/bmad-module/scripts/lib/legacy-resolver.mjs b/src/core-skills/bmad-module/scripts/lib/legacy-resolver.mjs index 959cbdd8e..5ba061e22 100644 --- a/src/core-skills/bmad-module/scripts/lib/legacy-resolver.mjs +++ b/src/core-skills/bmad-module/scripts/lib/legacy-resolver.mjs @@ -305,7 +305,7 @@ async function readModuleYaml(yamlAbs) { async function parseSkillFrontmatter(skillDirAbs) { try { const content = await fs.readFile(path.join(skillDirAbs, 'SKILL.md'), 'utf8'); - const match = content.match(/^---\s*\n([\s\S]*?)\n---/); + const match = content.match(/^---\s*\r?\n([\s\S]*?)\r?\n---/); if (!match) return { name: '', description: '' }; const parsed = parseYaml(match[1]) || {}; return { name: parsed.name || '', description: parsed.description || '' }; diff --git a/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/README.md b/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/README.md index d170ab2d2..fe1930a81 100644 --- a/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/README.md +++ b/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/README.md @@ -32,7 +32,7 @@ See `docs/index.md` (in this repo) for the customization recipe. ## Uninstall ``` -bmad-module remove devlog # leaves _bmad/custom/devlog/ intact +bmad-module remove devlog # leaves _bmad/custom/bmad-agent-historian*.toml intact bmad-module remove devlog --purge # removes user customizations too ``` diff --git a/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/scripts/fetch-git-history.sh b/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/scripts/fetch-git-history.sh index 2240105a4..d050347b4 100755 --- a/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/scripts/fetch-git-history.sh +++ b/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/scripts/fetch-git-history.sh @@ -22,8 +22,16 @@ if [ -f "$today_file" ]; then exit 0 fi -# Fall back to the most recent .md by mtime. -latest=$(ls -t "${devlog_path}"/*.md 2>/dev/null | head -n 1 || true) +# Fall back to the most recent .md by mtime. Glob + `-nt` instead of parsing +# `ls` so filenames with spaces/newlines are handled safely (and ShellCheck +# stays happy). +latest="" +for f in "${devlog_path}"/*.md; do + [ -e "$f" ] || continue + if [ -z "$latest" ] || [ "$f" -nt "$latest" ]; then + latest="$f" + fi +done if [ -n "$latest" ]; then echo "=== Most recent devlog ($(basename "$latest" .md)) ===" cat "$latest" diff --git a/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/skills/bmad-agent-historian/customize.toml b/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/skills/bmad-agent-historian/customize.toml index 3e1d4ddaa..caf579c55 100644 --- a/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/skills/bmad-agent-historian/customize.toml +++ b/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/skills/bmad-agent-historian/customize.toml @@ -22,8 +22,11 @@ activation_steps_append = [] # Persistent facts the agent keeps in mind for the whole session. # `file:` entries are loaded as content; literal entries are facts verbatim. +# Note: the devlog config is intentionally NOT loaded here as an always-on +# `file:` fact — it may not exist on first run, which would fail activation +# before the `/bmad-devlog-setup` fallback can create it. Skills read the +# config after checking it exists. persistent_facts = [ - "file:{project-root}/_bmad/devlog/config.yaml", "The devlog is a primary source. Never invent context that isn't written down.", ] diff --git a/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/skills/bmad-devlog-summarize/SKILL.md b/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/skills/bmad-devlog-summarize/SKILL.md index 969f90fb6..e00f04c1a 100644 --- a/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/skills/bmad-devlog-summarize/SKILL.md +++ b/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/skills/bmad-devlog-summarize/SKILL.md @@ -11,7 +11,7 @@ Walks devlog entries in a date range and produces a structured summary: themes, ### Step 1: Resolve config -Read `{project-root}/_bmad/devlog/config.yaml`. If missing, run `/bmad-devlog-setup` first. +Read `{project-root}/_bmad/devlog/config.yaml`. If missing, invoke `/bmad-devlog-setup` first. ### Step 2: Parse the range argument diff --git a/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/skills/bmad-devlog-write/SKILL.md b/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/skills/bmad-devlog-write/SKILL.md index 3a4583f8f..00d6b0fa8 100644 --- a/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/skills/bmad-devlog-write/SKILL.md +++ b/src/core-skills/bmad-module/tests/fixtures/examples/comprehensive/acme-devlog/skills/bmad-devlog-write/SKILL.md @@ -16,7 +16,7 @@ Read `{project-root}/_bmad/devlog/config.yaml`. Expect: - `devlog_path` (absolute path) - `entry_format` (`iso` | `weekly` | `monthly`) -If config is missing, run `/bmad-devlog-setup` first. +If config is missing, invoke `/bmad-devlog-setup` first. ### Step 2: Determine the entry file @@ -26,7 +26,7 @@ If config is missing, run `/bmad-devlog-setup` first. ### Step 3: Initialize if absent -If the file doesn't exist, copy `./assets/template.md` to the target path. Substitute `{{date}}`, `{{author}}` (from `user_name`), and `{{week}}`/`{{month}}` placeholders. +If the file doesn't exist, copy `./assets/template.md` to the target path. Substitute `{{date}}` and `{{author}}` (from `user_name`). For `weekly`/`monthly`, render the `{{date}}` heading from the period value (e.g. `2026-W21` or `2026-05`) instead of reusing the daily date. ### Step 4: Collect entry content diff --git a/src/core-skills/bmad-module/tests/integration.test.sh b/src/core-skills/bmad-module/tests/integration.test.sh index 66cfcccc9..6d818b3c6 100755 --- a/src/core-skills/bmad-module/tests/integration.test.sh +++ b/src/core-skills/bmad-module/tests/integration.test.sh @@ -34,11 +34,13 @@ ko() { printf ' \033[31m✗\033[0m %s\n' "$*"; fail=$((fail+1)); } # Wrapper that captures stdout/stderr/exit code into globals. run() { + local stderr_file + stderr_file="$(mktemp)" set +e - STDOUT="$(node "${MODULE_JS}" "$@" 2>/tmp/bmad-module-stderr.$$)" + STDOUT="$(node "${MODULE_JS}" "$@" 2>"${stderr_file}")" EXIT=$? - STDERR="$(cat /tmp/bmad-module-stderr.$$)" - rm -f /tmp/bmad-module-stderr.$$ + STDERR="$(cat "${stderr_file}")" + rm -f "${stderr_file}" set -e } @@ -115,15 +117,22 @@ ok "skeleton seeded at ${WORKDIR}/_bmad/" note "list (no modules)" run list assert_exit 0 "list empty" -[[ "${STDOUT}" == *"no modules installed"* ]] && ok "stdout reports empty" \ - || ko "expected 'no modules installed' in stdout: ${STDOUT}" +if [[ "${STDOUT}" == *"no modules installed"* ]]; then ok "stdout reports empty" +else ko "expected 'no modules installed' in stdout: ${STDOUT}"; fi + +# ─── 1a. usage errors: unknown flag / invalid channel reject early ─────────── +note "unknown flag and invalid channel → exit 2" +run install "${EXAMPLES}/minimal/acme-md-lint" --bogus +assert_exit 2 "unknown flag rejected" +run install "${EXAMPLES}/minimal/acme-md-lint" --channel stabl --dry-run +assert_exit 2 "invalid channel rejected" # ─── 2. dry-run install of minimal module ──────────────────────────────────── note "install --dry-run examples/minimal/acme-md-lint" run install "${EXAMPLES}/minimal/acme-md-lint" --dry-run assert_exit 0 "dry-run install" -[[ "${STDOUT}" == *"dry-run"* ]] && ok "stdout mentions dry-run" \ - || ko "expected 'dry-run' in stdout: ${STDOUT}" +if [[ "${STDOUT}" == *"dry-run"* ]]; then ok "stdout mentions dry-run" +else ko "expected 'dry-run' in stdout: ${STDOUT}"; fi assert_path_absent "_bmad/mdlint" # ─── 3. real install of minimal module ─────────────────────────────────────── @@ -141,13 +150,13 @@ assert_grep ',"mdlint",' "_bmad/_config/files-manifest.csv" note "list (after minimal install)" run list assert_exit 0 "list one" -[[ "${STDOUT}" == *"mdlint"* ]] && ok "stdout includes mdlint" \ - || ko "expected 'mdlint' in stdout: ${STDOUT}" +if [[ "${STDOUT}" == *"mdlint"* ]]; then ok "stdout includes mdlint" +else ko "expected 'mdlint' in stdout: ${STDOUT}"; fi run list --json assert_exit 0 "list --json" -[[ "${STDOUT}" == *"\"name\": \"mdlint\""* ]] && ok "json includes mdlint name" \ - || ko "expected mdlint in JSON: ${STDOUT}" +if [[ "${STDOUT}" == *"\"name\": \"mdlint\""* ]]; then ok "json includes mdlint name" +else ko "expected mdlint in JSON: ${STDOUT}"; fi # ─── 5. idempotent re-install ──────────────────────────────────────────────── note "install acme-md-lint again (idempotent / collision)" @@ -190,8 +199,8 @@ assert_path_exists "_bmad/devlog/module-help.csv" assert_path_absent "_bmad/devlog/docs" assert_path_absent "_bmad/devlog/README.md" assert_path_absent "_bmad/devlog/CHANGELOG.md" -[[ "${STDOUT}" == *"hooks"* ]] && ok "warns about hooks not auto-activated" \ - || ko "expected hooks warning in stdout: ${STDOUT}" +if [[ "${STDOUT}" == *"hooks"* ]]; then ok "warns about hooks not auto-activated" +else ko "expected hooks warning in stdout: ${STDOUT}"; fi # ─── 9a. parity: central config + agent roster (gap #3) ────────────────────── note "config generation + agent roster" @@ -213,8 +222,9 @@ assert_path_exists "_bmad-output/journal" # ─── 9c. parity: merged help catalog (gap #1) ──────────────────────────────── note "bmad-help.csv merge" assert_path_exists "_bmad/_config/bmad-help.csv" -head -1 _bmad/_config/bmad-help.csv | grep -q '^module,skill,display-name,' \ - && ok "bmad-help.csv has canonical header" || ko "bmad-help.csv header wrong" +if head -1 _bmad/_config/bmad-help.csv | grep -q '^module,skill,display-name,'; then + ok "bmad-help.csv has canonical header" +else ko "bmad-help.csv header wrong"; fi assert_grep '^devlog,bmad-devlog-write,' "_bmad/_config/bmad-help.csv" assert_grep '^devlog,bmad-agent-historian,' "_bmad/_config/bmad-help.csv" # the core baseline row is still present @@ -224,8 +234,8 @@ assert_grep ',bmad-help,Help,' "_bmad/_config/bmad-help.csv" note "install examples/legacy/bmad-mini-legacy (legacy marketplace.json)" run install "${EXAMPLES}/legacy/bmad-mini-legacy" assert_exit 0 "install legacy mini" -[[ "${STDOUT}" == *"resolved legacy module mlg"* ]] && ok "stdout reports legacy resolution" \ - || ko "expected 'resolved legacy module mlg' in stdout: ${STDOUT}" +if [[ "${STDOUT}" == *"resolved legacy module mlg"* ]]; then ok "stdout reports legacy resolution" +else ko "expected 'resolved legacy module mlg' in stdout: ${STDOUT}"; fi # Synthetic plugin.json is staged; marketplace.json is preserved verbatim. assert_path_exists "_bmad/mlg/.claude-plugin/plugin.json" assert_path_exists "_bmad/mlg/.claude-plugin/marketplace.json" @@ -273,13 +283,15 @@ run remove mdlint assert_exit 0 "remove mdlint" assert_path_absent "_bmad/mdlint" assert_path_exists "_bmad/custom/mdlint/override.md" -[[ "${STDOUT}" == *"preserved"* ]] && ok "stdout mentions preserved customs" \ - || ko "expected 'preserved' in stdout: ${STDOUT}" +if [[ "${STDOUT}" == *"preserved"* ]]; then ok "stdout mentions preserved customs" +else ko "expected 'preserved' in stdout: ${STDOUT}"; fi # manifest rows for mdlint should be gone -grep -q ',"mdlint",' _bmad/_config/files-manifest.csv && \ - ko "mdlint rows still in files-manifest.csv" || ok "files-manifest.csv pruned" -grep -q '"acme-md-lint"' _bmad/_config/skill-manifest.csv && \ - ko "acme-md-lint row still in skill-manifest.csv" || ok "skill-manifest.csv pruned" +if grep -q ',"mdlint",' _bmad/_config/files-manifest.csv; then + ko "mdlint rows still in files-manifest.csv" +else ok "files-manifest.csv pruned"; fi +if grep -q '"acme-md-lint"' _bmad/_config/skill-manifest.csv; then + ko "acme-md-lint row still in skill-manifest.csv" +else ok "skill-manifest.csv pruned"; fi # ─── 11. remove --purge ────────────────────────────────────────────────────── note "remove devlog --purge" @@ -290,14 +302,18 @@ assert_exit 0 "remove --purge" assert_path_absent "_bmad/devlog" assert_path_absent "_bmad/custom/devlog" # config blocks and help rows for devlog are stripped on removal -grep -q '\[modules\.devlog]' _bmad/config.toml \ - && ko "[modules.devlog] still in config.toml" || ok "config.toml [modules.devlog] stripped" -grep -q '\[agents\.bmad-agent-historian]' _bmad/config.toml \ - && ko "[agents.bmad-agent-historian] still in config.toml" || ok "config.toml agent block stripped" -grep -q '\[modules\.devlog]' _bmad/config.user.toml \ - && ko "[modules.devlog] still in config.user.toml" || ok "config.user.toml [modules.devlog] stripped" -grep -q '^devlog,' _bmad/_config/bmad-help.csv \ - && ko "devlog rows still in bmad-help.csv" || ok "bmad-help.csv devlog rows removed" +if grep -q '\[modules\.devlog]' _bmad/config.toml; then + ko "[modules.devlog] still in config.toml" +else ok "config.toml [modules.devlog] stripped"; fi +if grep -q '\[agents\.bmad-agent-historian]' _bmad/config.toml; then + ko "[agents.bmad-agent-historian] still in config.toml" +else ok "config.toml agent block stripped"; fi +if grep -q '\[modules\.devlog]' _bmad/config.user.toml; then + ko "[modules.devlog] still in config.user.toml" +else ok "config.user.toml [modules.devlog] stripped"; fi +if grep -q '^devlog,' _bmad/_config/bmad-help.csv; then + ko "devlog rows still in bmad-help.csv" +else ok "bmad-help.csv devlog rows removed"; fi # [core] survives the removal assert_grep '^user_name = "Tester"' "_bmad/config.toml" @@ -330,8 +346,8 @@ run install "${EXAMPLES}/minimal/acme-md-lint" --project-dir "${IDEPROJ}" assert_exit 0 "install into IDE project" assert_path_exists "${IDEPROJ}/.claude/skills/acme-md-lint/SKILL.md" assert_path_exists "${IDEPROJ}/.agents/skills/acme-md-lint/SKILL.md" -[[ "${STDOUT}" == *"claude-code"* ]] && ok "stdout reports claude-code distribution" \ - || ko "expected claude-code in stdout: ${STDOUT}" +if [[ "${STDOUT}" == *"claude-code"* ]]; then ok "stdout reports claude-code distribution" +else ko "expected claude-code in stdout: ${STDOUT}"; fi # Canonical end-state: skill source dirs removed from _bmad/ after distribution. if find "${IDEPROJ}/_bmad" -name SKILL.md | grep -q .; then ko "SKILL.md still under _bmad after distribution" @@ -354,9 +370,9 @@ run install "${EXAMPLES}/minimal-npm/acme-npmtool" assert_exit 0 "install npm fixture" assert_path_exists "_bmad/npmtool/package.json" if command -v npm >/dev/null 2>&1; then - [[ "${STDOUT}" == *"installed npm dependencies for npmtool"* ]] \ - && ok "npm dependencies installed" \ - || ko "expected npm install confirmation in stdout: ${STDOUT}" + if [[ "${STDOUT}" == *"installed npm dependencies for npmtool"* ]]; then + ok "npm dependencies installed" + else ko "expected npm install confirmation in stdout: ${STDOUT}"; fi # The fixture has zero deps, so npm writes package-lock.json (not node_modules); # its presence proves npm actually ran inside the installed module dir. assert_path_exists "_bmad/npmtool/package-lock.json" diff --git a/test/test-installation-components.js b/test/test-installation-components.js index c920f3a1d..103416987 100644 --- a/test/test-installation-components.js +++ b/test/test-installation-components.js @@ -3739,8 +3739,12 @@ async function runTests() { const om = new OfficialModules(); const tracked = []; - const result = await om.install('devlog', bmadDir, (p) => tracked.push(p), { skipModuleInstaller: true, moduleConfig: {} }); - CustomModuleManager._resolutionCache.delete('devlog'); + let result; + try { + result = await om.install('devlog', bmadDir, (p) => tracked.push(p), { skipModuleInstaller: true, moduleConfig: {} }); + } finally { + CustomModuleManager._resolutionCache.delete('devlog'); + } assert(result.success === true && result.module === 'devlog', 'install() routes plugin-json resolution and succeeds'); assert(