diff --git a/docs/explanation/module-configuration.md b/docs/explanation/module-configuration.md index 36dedf9..8aff28b 100644 --- a/docs/explanation/module-configuration.md +++ b/docs/explanation/module-configuration.md @@ -98,6 +98,12 @@ When the setup skill runs, it merges these rows into the project-wide `_bmad/mod Both merge scripts use an anti-zombie pattern: before writing new values for a module, they remove all existing entries for that module's code. This prevents stale configuration or help entries from persisting across module updates. Running setup a second time is always safe. +## Legacy Directory Cleanup + +After config data is migrated and individual files are cleaned up by the merge scripts, a separate cleanup step removes the installer's per-module directory trees from `_bmad/`. These directories contain skill files that are already installed at `.claude/skills/` — they are redundant once the config has been consolidated. + +Before removing any directory, the cleanup script verifies that every skill it contains exists at the installed location. Directories without skills (like `_config/`) are removed directly. The script is idempotent — running setup again after cleanup is safe. + ## Design Guidance Configuration is for **basic, project-level settings** — output folders, language preferences, feature toggles. Keep the number of configurable values small. diff --git a/src/skills/bmad-builder-setup/SKILL.md b/src/skills/bmad-builder-setup/SKILL.md index 4365ced..b02837e 100644 --- a/src/skills/bmad-builder-setup/SKILL.md +++ b/src/skills/bmad-builder-setup/SKILL.md @@ -7,10 +7,10 @@ description: Sets up BMad Builder module in a project. Use when the user request ## Overview -Installs and configures a BMad module into a project. Module identity (name, code, version) comes from `assets/module.yaml`. Collects user preferences and writes them to three files: +Installs and configures a BMad module into a project. Module identity (name, code, version) comes from `./assets/module.yaml`. Collects user preferences and writes them to three files: - **`{project-root}/_bmad/config.yaml`** — shared project config: core settings at root (e.g. `output_folder`, `document_output_language`) plus a section per module with metadata and module-specific values. User-only keys (`user_name`, `communication_language`) are **never** written here. -- **`{project-root}/_bmad/config.user.yaml`** — personal settings intended to be gitignored: `user_name`, `communication_language`, and any module variable marked `user_setting: true` in `assets/module.yaml`. These values live exclusively here. +- **`{project-root}/_bmad/config.user.yaml`** — personal settings intended to be gitignored: `user_name`, `communication_language`, and any module variable marked `user_setting: true` in `./assets/module.yaml`. These values live exclusively here. - **`{project-root}/_bmad/module-help.csv`** — registers module capabilities for the help system. Both config scripts use an anti-zombie pattern — existing entries for this module are removed before writing fresh ones, so stale values never persist. @@ -21,36 +21,55 @@ Both config scripts use an anti-zombie pattern — existing entries for this mod 1. Read `./assets/module.yaml` for module metadata and variable definitions (the `code` field is the module identifier) 2. Check if `{project-root}/_bmad/config.yaml` exists — if a section matching the module's code is already present, inform the user this is an update -3. Check for legacy configuration at `{project-root}/_bmad/{module-code}/config.yaml` and `{project-root}/_bmad/core/config.yaml`. If either file exists, inform the user that legacy values will be migrated and the old files removed after setup +3. Check for per-module configuration at `{project-root}/_bmad/{module-code}/config.yaml` and `{project-root}/_bmad/core/config.yaml`. If either file exists: + - If `{project-root}/_bmad/config.yaml` does **not** yet have a section for this module: this is a **fresh install**. Inform the user that installer config was detected and values will be consolidated into the new format. + - If `{project-root}/_bmad/config.yaml` **already** has a section for this module: this is a **legacy migration**. Inform the user that legacy per-module config was found alongside existing config, and legacy values will be used as fallback defaults. + - In both cases, per-module config files and directories will be cleaned up after setup. If the user provides arguments (e.g. `accept all defaults`, `--headless`, or inline values like `user name is BMad, I speak Swahili`), map any provided values to config keys, use defaults for the rest, and skip interactive prompting. Still display the full confirmation summary at the end. ## Collect Configuration -Ask the user for values. Show defaults in brackets. Present all values together so the user can respond once with only the values they want to change (e.g. "change language to Swahili, rest are fine"). Use a structured question tool if available. Never tell the user to "press enter" or "leave blank" — in a chat interface they must type something to respond. +Ask the user for values. Show defaults in brackets. Present all values together so the user can respond once with only the values they want to change (e.g. "change language to Swahili, rest are fine"). Never tell the user to "press enter" or "leave blank" — in a chat interface they must type something to respond. -**Default priority** (highest wins): existing new config values > legacy config values > `assets/module.yaml` defaults. When legacy configs exist, read them and use matching values as defaults instead of `module.yaml` defaults. Only keys that match the current schema are carried forward — changed or removed keys are ignored. +**Default priority** (highest wins): existing new config values > legacy config values > `./assets/module.yaml` defaults. When legacy configs exist, read them and use matching values as defaults instead of `module.yaml` defaults. Only keys that match the current schema are carried forward — changed or removed keys are ignored. -**Core config** (only if no core keys exist yet): `user_name` (default: BMad), `communication_language` (default: English), `document_output_language` (default: English), `output_folder` (default: `{project-root}/_bmad-output`). Of these, `user_name` and `communication_language` are written exclusively to `config.user.yaml`. The rest go to `config.yaml` at root and are shared across all modules. +**Core config** (only if no core keys exist yet): `user_name` (default: BMad), `communication_language` and `document_output_language` (default: English — ask as a single language question, both keys get the same answer), `output_folder` (default: `{project-root}/_bmad-output`). Of these, `user_name` and `communication_language` are written exclusively to `config.user.yaml`. The rest go to `config.yaml` at root and are shared across all modules. -**Module config**: Read each variable in `assets/module.yaml` that has a `prompt` field. Ask using that prompt with its default value (or legacy value if available). +**Module config**: Read each variable in `./assets/module.yaml` that has a `prompt` field. Ask using that prompt with its default value (or legacy value if available). ## Write Files Write a temp JSON file with the collected answers structured as `{"core": {...}, "module": {...}}` (omit `core` if it already exists). Then run both scripts — they can run in parallel since they write to different files: ```bash -python3 ./scripts/merge-config.py --config-path "{project-root}/_bmad/config.yaml" --user-config-path "{project-root}/_bmad/config.user.yaml" --module-yaml assets/module.yaml --answers {temp-file} --legacy-dir "{project-root}/_bmad" -python3 ./scripts/merge-help-csv.py --target "{project-root}/_bmad/module-help.csv" --source assets/module-help.csv --legacy-dir "{project-root}/_bmad" --module-code {module-code} +python3 ./scripts/merge-config.py --config-path "{project-root}/_bmad/config.yaml" --user-config-path "{project-root}/_bmad/config.user.yaml" --module-yaml ./assets/module.yaml --answers {temp-file} --legacy-dir "{project-root}/_bmad" +python3 ./scripts/merge-help-csv.py --target "{project-root}/_bmad/module-help.csv" --source ./assets/module-help.csv --legacy-dir "{project-root}/_bmad" --module-code {module-code} ``` Both scripts output JSON to stdout with results. If either exits non-zero, surface the error and stop. The scripts automatically read legacy config values as fallback defaults, then delete the legacy files after a successful merge. Check `legacy_configs_deleted` and `legacy_csvs_deleted` in the output to confirm cleanup. -Run `./scripts/merge-config.py --help` or `scripts/merge-help-csv.py --help` for full usage. +Run `./scripts/merge-config.py --help` or `./scripts/merge-help-csv.py --help` for full usage. + +## Create Output Directories + +After writing config, create any output directories that were configured. For filesystem operations only (such as creating directories), resolve the `{project-root}` token to the actual project root and create each path-type value from `config.yaml` that does not yet exist — this includes `output_folder` and any module variable whose value starts with `{project-root}/`. The paths stored in the config files must continue to use the literal `{project-root}` token; only the directories on disk should use the resolved paths. Use `mkdir -p` or equivalent to create the full path. + +## Cleanup Legacy Directories + +After both merge scripts complete successfully, remove the installer's package directories. Skills and agents in these directories are already installed at `.claude/skills/` — the `_bmad/` directory should only contain config files. + +```bash +python3 ./scripts/cleanup-legacy.py --bmad-dir "{project-root}/_bmad" --module-code {module-code} --also-remove _config --skills-dir "{project-root}/.claude/skills" +``` + +The script verifies that every skill in the legacy directories exists at `.claude/skills/` before removing anything. Directories without skills (like `_config/`) are removed directly. If the script exits non-zero, surface the error and stop. Missing directories (already cleaned by a prior run) are not errors — the script is idempotent. + +Check `directories_removed` and `files_removed_count` in the JSON output for the confirmation step. Run `./scripts/cleanup-legacy.py --help` for full usage. ## Confirm -Use the script JSON output to display what was written — config values set (written to `config.yaml` at root for core, module section for module values), user settings written to `config.user.yaml` (`user_keys` in result), help entries added, fresh install vs update. If legacy files were deleted, mention the migration. Then display the `module_greeting` from `assets/module.yaml` to the user. +Use the script JSON output to display what was written — config values set (written to `config.yaml` at root for core, module section for module values), user settings written to `config.user.yaml` (`user_keys` in result), help entries added, fresh install vs update. If legacy files were deleted, mention the migration. If legacy directories were removed, report the count and list (e.g. "Cleaned up 106 installer package files from bmb/, core/, _config/ — skills are installed at .claude/skills/"). Then display the `module_greeting` from `./assets/module.yaml` to the user. ## Outcome diff --git a/src/skills/bmad-builder-setup/scripts/cleanup-legacy.py b/src/skills/bmad-builder-setup/scripts/cleanup-legacy.py new file mode 100755 index 0000000..fc12f40 --- /dev/null +++ b/src/skills/bmad-builder-setup/scripts/cleanup-legacy.py @@ -0,0 +1,259 @@ +#!/usr/bin/env python3 +# /// script +# requires-python = ">=3.9" +# dependencies = [] +# /// +"""Remove legacy module directories from _bmad/ after config migration. + +After merge-config.py and merge-help-csv.py have migrated config data and +deleted individual legacy files, this script removes the now-redundant +directory trees. These directories contain skill files that are already +installed at .claude/skills/ (or equivalent) — only the config files at +_bmad/ root need to persist. + +When --skills-dir is provided, the script verifies that every skill found +in the legacy directories exists at the installed location before removing +anything. Directories without skills (like _config/) are removed directly. + +Exit codes: 0=success (including nothing to remove), 1=validation error, 2=runtime error +""" + +import argparse +import json +import shutil +import sys +from pathlib import Path + + +def parse_args(): + parser = argparse.ArgumentParser( + description="Remove legacy module directories from _bmad/ after config migration." + ) + parser.add_argument( + "--bmad-dir", + required=True, + help="Path to the _bmad/ directory", + ) + parser.add_argument( + "--module-code", + required=True, + help="Module code being cleaned up (e.g. 'bmb')", + ) + parser.add_argument( + "--also-remove", + action="append", + default=[], + help="Additional directory names under _bmad/ to remove (repeatable)", + ) + parser.add_argument( + "--skills-dir", + help="Path to .claude/skills/ — enables safety verification that skills " + "are installed before removing legacy copies", + ) + parser.add_argument( + "--verbose", + action="store_true", + help="Print detailed progress to stderr", + ) + return parser.parse_args() + + +def find_skill_dirs(base_path: str) -> list: + """Find directories that contain a SKILL.md file. + + Walks the directory tree and returns the leaf directory name for each + directory containing a SKILL.md. These are considered skill directories. + + Returns: + List of skill directory names (e.g. ['bmad-agent-builder', 'bmad-builder-setup']) + """ + skills = [] + root = Path(base_path) + if not root.exists(): + return skills + for skill_md in root.rglob("SKILL.md"): + skills.append(skill_md.parent.name) + return sorted(set(skills)) + + +def verify_skills_installed( + bmad_dir: str, dirs_to_check: list, skills_dir: str, verbose: bool = False +) -> list: + """Verify that skills in legacy directories exist at the installed location. + + Scans each directory in dirs_to_check for skill folders (containing SKILL.md), + then checks that a matching directory exists under skills_dir. Directories + that contain no skills (like _config/) are silently skipped. + + Returns: + List of verified skill names. + + Raises SystemExit(1) if any skills are missing from skills_dir. + """ + all_verified = [] + missing = [] + + for dirname in dirs_to_check: + legacy_path = Path(bmad_dir) / dirname + if not legacy_path.exists(): + continue + + skill_names = find_skill_dirs(str(legacy_path)) + if not skill_names: + if verbose: + print( + f"No skills found in {dirname}/ — skipping verification", + file=sys.stderr, + ) + continue + + for skill_name in skill_names: + installed_path = Path(skills_dir) / skill_name + if installed_path.is_dir(): + all_verified.append(skill_name) + if verbose: + print( + f"Verified: {skill_name} exists at {installed_path}", + file=sys.stderr, + ) + else: + missing.append(skill_name) + if verbose: + print( + f"MISSING: {skill_name} not found at {installed_path}", + file=sys.stderr, + ) + + if missing: + error_result = { + "status": "error", + "error": "Skills not found at installed location", + "missing_skills": missing, + "skills_dir": str(Path(skills_dir).resolve()), + } + print(json.dumps(error_result, indent=2)) + sys.exit(1) + + return sorted(set(all_verified)) + + +def count_files(path: Path) -> int: + """Count all files recursively in a directory.""" + count = 0 + for item in path.rglob("*"): + if item.is_file(): + count += 1 + return count + + +def cleanup_directories( + bmad_dir: str, dirs_to_remove: list, verbose: bool = False +) -> tuple: + """Remove specified directories under bmad_dir. + + Returns: + (removed, not_found, total_files_removed) tuple + """ + removed = [] + not_found = [] + total_files = 0 + + for dirname in dirs_to_remove: + target = Path(bmad_dir) / dirname + if not target.exists(): + not_found.append(dirname) + if verbose: + print(f"Not found (skipping): {target}", file=sys.stderr) + continue + + if not target.is_dir(): + if verbose: + print(f"Not a directory (skipping): {target}", file=sys.stderr) + not_found.append(dirname) + continue + + file_count = count_files(target) + if verbose: + print( + f"Removing {target} ({file_count} files)", + file=sys.stderr, + ) + + try: + shutil.rmtree(target) + except OSError as e: + error_result = { + "status": "error", + "error": f"Failed to remove {target}: {e}", + "directories_removed": removed, + "directories_failed": dirname, + } + print(json.dumps(error_result, indent=2)) + sys.exit(2) + + removed.append(dirname) + total_files += file_count + + return removed, not_found, total_files + + +def main(): + args = parse_args() + + bmad_dir = args.bmad_dir + module_code = args.module_code + + # Build the list of directories to remove + dirs_to_remove = [module_code, "core"] + args.also_remove + # Deduplicate while preserving order + seen = set() + unique_dirs = [] + for d in dirs_to_remove: + if d not in seen: + seen.add(d) + unique_dirs.append(d) + dirs_to_remove = unique_dirs + + if args.verbose: + print(f"Directories to remove: {dirs_to_remove}", file=sys.stderr) + + # Safety check: verify skills are installed before removing + verified_skills = None + if args.skills_dir: + if args.verbose: + print( + f"Verifying skills installed at {args.skills_dir}", + file=sys.stderr, + ) + verified_skills = verify_skills_installed( + bmad_dir, dirs_to_remove, args.skills_dir, args.verbose + ) + + # Remove directories + removed, not_found, total_files = cleanup_directories( + bmad_dir, dirs_to_remove, args.verbose + ) + + # Build result + result = { + "status": "success", + "bmad_dir": str(Path(bmad_dir).resolve()), + "directories_removed": removed, + "directories_not_found": not_found, + "files_removed_count": total_files, + } + + if args.skills_dir: + result["safety_checks"] = { + "skills_verified": True, + "skills_dir": str(Path(args.skills_dir).resolve()), + "verified_skills": verified_skills, + } + else: + result["safety_checks"] = None + + print(json.dumps(result, indent=2)) + + +if __name__ == "__main__": + main() diff --git a/src/skills/bmad-builder-setup/scripts/merge-config.py b/src/skills/bmad-builder-setup/scripts/merge-config.py index 5089cbc..6ee0ac7 100755 --- a/src/skills/bmad-builder-setup/scripts/merge-config.py +++ b/src/skills/bmad-builder-setup/scripts/merge-config.py @@ -191,6 +191,36 @@ def extract_module_metadata(module_yaml: dict) -> dict: return meta +def apply_result_templates( + module_yaml: dict, module_answers: dict, verbose: bool = False +) -> dict: + """Apply result templates from module.yaml to transform raw answer values. + + For each answer, if the corresponding variable definition in module.yaml has + a 'result' field, replaces {value} in that template with the answer. Skips + the template if the answer already contains '{project-root}' to prevent + double-prefixing. + """ + transformed = {} + for key, value in module_answers.items(): + var_def = module_yaml.get(key) + if ( + isinstance(var_def, dict) + and "result" in var_def + and "{project-root}" not in str(value) + ): + template = var_def["result"] + transformed[key] = template.replace("{value}", str(value)) + if verbose: + print( + f"Applied result template for '{key}': {value} → {transformed[key]}", + file=sys.stderr, + ) + else: + transformed[key] = value + return transformed + + def merge_config( existing_config: dict, module_yaml: dict, @@ -249,7 +279,9 @@ def merge_config( # Build module section: metadata + variable values module_section = extract_module_metadata(module_yaml) - module_answers = answers.get("module", {}) + module_answers = apply_result_templates( + module_yaml, answers.get("module", {}), verbose + ) module_section.update(module_answers) if verbose: diff --git a/src/skills/bmad-builder-setup/scripts/tests/test-cleanup-legacy.py b/src/skills/bmad-builder-setup/scripts/tests/test-cleanup-legacy.py new file mode 100644 index 0000000..f481e51 --- /dev/null +++ b/src/skills/bmad-builder-setup/scripts/tests/test-cleanup-legacy.py @@ -0,0 +1,429 @@ +#!/usr/bin/env python3 +# /// script +# requires-python = ">=3.9" +# dependencies = [] +# /// +"""Unit tests for cleanup-legacy.py.""" + +import json +import os +import sys +import tempfile +import unittest +from pathlib import Path + +# Add parent directory to path so we can import the module +sys.path.insert(0, str(Path(__file__).parent.parent)) + +from importlib.util import spec_from_file_location, module_from_spec + +# Import cleanup_legacy module +_spec = spec_from_file_location( + "cleanup_legacy", + str(Path(__file__).parent.parent / "cleanup-legacy.py"), +) +cleanup_legacy_mod = module_from_spec(_spec) +_spec.loader.exec_module(cleanup_legacy_mod) + +find_skill_dirs = cleanup_legacy_mod.find_skill_dirs +verify_skills_installed = cleanup_legacy_mod.verify_skills_installed +count_files = cleanup_legacy_mod.count_files +cleanup_directories = cleanup_legacy_mod.cleanup_directories + + +def _make_skill_dir(base, *path_parts): + """Create a skill directory with a SKILL.md file.""" + skill_dir = os.path.join(base, *path_parts) + os.makedirs(skill_dir, exist_ok=True) + with open(os.path.join(skill_dir, "SKILL.md"), "w") as f: + f.write("---\nname: test-skill\n---\n# Test\n") + return skill_dir + + +def _make_file(base, *path_parts, content="placeholder"): + """Create a file at the given path.""" + file_path = os.path.join(base, *path_parts) + os.makedirs(os.path.dirname(file_path), exist_ok=True) + with open(file_path, "w") as f: + f.write(content) + return file_path + + +class TestFindSkillDirs(unittest.TestCase): + def test_finds_dirs_with_skill_md(self): + with tempfile.TemporaryDirectory() as tmpdir: + _make_skill_dir(tmpdir, "skills", "bmad-agent-builder") + _make_skill_dir(tmpdir, "skills", "bmad-workflow-builder") + result = find_skill_dirs(tmpdir) + self.assertEqual(result, ["bmad-agent-builder", "bmad-workflow-builder"]) + + def test_ignores_dirs_without_skill_md(self): + with tempfile.TemporaryDirectory() as tmpdir: + _make_skill_dir(tmpdir, "skills", "real-skill") + os.makedirs(os.path.join(tmpdir, "skills", "not-a-skill")) + _make_file(tmpdir, "skills", "not-a-skill", "README.md") + result = find_skill_dirs(tmpdir) + self.assertEqual(result, ["real-skill"]) + + def test_empty_directory(self): + with tempfile.TemporaryDirectory() as tmpdir: + result = find_skill_dirs(tmpdir) + self.assertEqual(result, []) + + def test_nonexistent_directory(self): + result = find_skill_dirs("/nonexistent/path") + self.assertEqual(result, []) + + def test_finds_nested_skills_in_phase_subdirs(self): + """Skills nested in phase directories like bmm/1-analysis/bmad-agent-analyst/.""" + with tempfile.TemporaryDirectory() as tmpdir: + _make_skill_dir(tmpdir, "1-analysis", "bmad-agent-analyst") + _make_skill_dir(tmpdir, "2-plan", "bmad-agent-pm") + _make_skill_dir(tmpdir, "4-impl", "bmad-agent-dev") + result = find_skill_dirs(tmpdir) + self.assertEqual( + result, ["bmad-agent-analyst", "bmad-agent-dev", "bmad-agent-pm"] + ) + + def test_deduplicates_skill_names(self): + """If the same skill name appears in multiple locations, only listed once.""" + with tempfile.TemporaryDirectory() as tmpdir: + _make_skill_dir(tmpdir, "a", "my-skill") + _make_skill_dir(tmpdir, "b", "my-skill") + result = find_skill_dirs(tmpdir) + self.assertEqual(result, ["my-skill"]) + + +class TestVerifySkillsInstalled(unittest.TestCase): + def test_all_skills_present(self): + with tempfile.TemporaryDirectory() as tmpdir: + bmad_dir = os.path.join(tmpdir, "_bmad") + skills_dir = os.path.join(tmpdir, "skills") + + # Legacy: bmb has two skills + _make_skill_dir(bmad_dir, "bmb", "skills", "skill-a") + _make_skill_dir(bmad_dir, "bmb", "skills", "skill-b") + + # Installed: both exist + os.makedirs(os.path.join(skills_dir, "skill-a")) + os.makedirs(os.path.join(skills_dir, "skill-b")) + + result = verify_skills_installed(bmad_dir, ["bmb"], skills_dir) + self.assertEqual(result, ["skill-a", "skill-b"]) + + def test_missing_skill_exits_1(self): + with tempfile.TemporaryDirectory() as tmpdir: + bmad_dir = os.path.join(tmpdir, "_bmad") + skills_dir = os.path.join(tmpdir, "skills") + + _make_skill_dir(bmad_dir, "bmb", "skills", "skill-a") + _make_skill_dir(bmad_dir, "bmb", "skills", "skill-missing") + + # Only skill-a installed + os.makedirs(os.path.join(skills_dir, "skill-a")) + + with self.assertRaises(SystemExit) as ctx: + verify_skills_installed(bmad_dir, ["bmb"], skills_dir) + self.assertEqual(ctx.exception.code, 1) + + def test_empty_legacy_dir_passes(self): + with tempfile.TemporaryDirectory() as tmpdir: + bmad_dir = os.path.join(tmpdir, "_bmad") + skills_dir = os.path.join(tmpdir, "skills") + os.makedirs(bmad_dir) + os.makedirs(skills_dir) + + result = verify_skills_installed(bmad_dir, ["bmb"], skills_dir) + self.assertEqual(result, []) + + def test_nonexistent_legacy_dir_skipped(self): + with tempfile.TemporaryDirectory() as tmpdir: + bmad_dir = os.path.join(tmpdir, "_bmad") + skills_dir = os.path.join(tmpdir, "skills") + os.makedirs(skills_dir) + # bmad_dir doesn't exist — should not error + + result = verify_skills_installed(bmad_dir, ["bmb"], skills_dir) + self.assertEqual(result, []) + + def test_dir_without_skills_skipped(self): + """Directories like _config/ that have no SKILL.md are not verified.""" + with tempfile.TemporaryDirectory() as tmpdir: + bmad_dir = os.path.join(tmpdir, "_bmad") + skills_dir = os.path.join(tmpdir, "skills") + + # _config has files but no SKILL.md + _make_file(bmad_dir, "_config", "manifest.yaml", content="version: 1") + _make_file(bmad_dir, "_config", "help.csv", content="a,b,c") + os.makedirs(skills_dir) + + result = verify_skills_installed(bmad_dir, ["_config"], skills_dir) + self.assertEqual(result, []) + + def test_verifies_across_multiple_dirs(self): + with tempfile.TemporaryDirectory() as tmpdir: + bmad_dir = os.path.join(tmpdir, "_bmad") + skills_dir = os.path.join(tmpdir, "skills") + + _make_skill_dir(bmad_dir, "bmb", "skills", "skill-a") + _make_skill_dir(bmad_dir, "core", "skills", "skill-b") + + os.makedirs(os.path.join(skills_dir, "skill-a")) + os.makedirs(os.path.join(skills_dir, "skill-b")) + + result = verify_skills_installed( + bmad_dir, ["bmb", "core"], skills_dir + ) + self.assertEqual(result, ["skill-a", "skill-b"]) + + +class TestCountFiles(unittest.TestCase): + def test_counts_files_recursively(self): + with tempfile.TemporaryDirectory() as tmpdir: + _make_file(tmpdir, "a.txt") + _make_file(tmpdir, "sub", "b.txt") + _make_file(tmpdir, "sub", "deep", "c.txt") + self.assertEqual(count_files(Path(tmpdir)), 3) + + def test_empty_dir_returns_zero(self): + with tempfile.TemporaryDirectory() as tmpdir: + self.assertEqual(count_files(Path(tmpdir)), 0) + + +class TestCleanupDirectories(unittest.TestCase): + def test_removes_single_module_dir(self): + with tempfile.TemporaryDirectory() as tmpdir: + bmad_dir = os.path.join(tmpdir, "_bmad") + os.makedirs(os.path.join(bmad_dir, "bmb", "skills")) + _make_file(bmad_dir, "bmb", "skills", "SKILL.md") + + removed, not_found, count = cleanup_directories(bmad_dir, ["bmb"]) + self.assertEqual(removed, ["bmb"]) + self.assertEqual(not_found, []) + self.assertGreater(count, 0) + self.assertFalse(os.path.exists(os.path.join(bmad_dir, "bmb"))) + + def test_removes_module_core_and_config(self): + with tempfile.TemporaryDirectory() as tmpdir: + bmad_dir = os.path.join(tmpdir, "_bmad") + for dirname in ("bmb", "core", "_config"): + _make_file(bmad_dir, dirname, "some-file.txt") + + removed, not_found, count = cleanup_directories( + bmad_dir, ["bmb", "core", "_config"] + ) + self.assertEqual(sorted(removed), ["_config", "bmb", "core"]) + self.assertEqual(not_found, []) + for dirname in ("bmb", "core", "_config"): + self.assertFalse(os.path.exists(os.path.join(bmad_dir, dirname))) + + def test_nonexistent_dir_in_not_found(self): + with tempfile.TemporaryDirectory() as tmpdir: + bmad_dir = os.path.join(tmpdir, "_bmad") + os.makedirs(bmad_dir) + + removed, not_found, count = cleanup_directories(bmad_dir, ["bmb"]) + self.assertEqual(removed, []) + self.assertEqual(not_found, ["bmb"]) + self.assertEqual(count, 0) + + def test_preserves_other_module_dirs(self): + with tempfile.TemporaryDirectory() as tmpdir: + bmad_dir = os.path.join(tmpdir, "_bmad") + for dirname in ("bmb", "bmm", "tea"): + _make_file(bmad_dir, dirname, "file.txt") + + removed, not_found, count = cleanup_directories(bmad_dir, ["bmb"]) + self.assertEqual(removed, ["bmb"]) + self.assertTrue(os.path.isdir(os.path.join(bmad_dir, "bmm"))) + self.assertTrue(os.path.isdir(os.path.join(bmad_dir, "tea"))) + + def test_preserves_root_config_files(self): + with tempfile.TemporaryDirectory() as tmpdir: + bmad_dir = os.path.join(tmpdir, "_bmad") + _make_file(bmad_dir, "config.yaml", content="key: val") + _make_file(bmad_dir, "config.user.yaml", content="user: test") + _make_file(bmad_dir, "module-help.csv", content="a,b,c") + _make_file(bmad_dir, "bmb", "stuff.txt") + + cleanup_directories(bmad_dir, ["bmb"]) + + self.assertTrue(os.path.exists(os.path.join(bmad_dir, "config.yaml"))) + self.assertTrue( + os.path.exists(os.path.join(bmad_dir, "config.user.yaml")) + ) + self.assertTrue( + os.path.exists(os.path.join(bmad_dir, "module-help.csv")) + ) + + def test_removes_hidden_files(self): + with tempfile.TemporaryDirectory() as tmpdir: + bmad_dir = os.path.join(tmpdir, "_bmad") + _make_file(bmad_dir, "bmb", ".DS_Store") + _make_file(bmad_dir, "bmb", "skills", ".hidden") + + removed, not_found, count = cleanup_directories(bmad_dir, ["bmb"]) + self.assertEqual(removed, ["bmb"]) + self.assertEqual(count, 2) + self.assertFalse(os.path.exists(os.path.join(bmad_dir, "bmb"))) + + def test_idempotent_rerun(self): + with tempfile.TemporaryDirectory() as tmpdir: + bmad_dir = os.path.join(tmpdir, "_bmad") + _make_file(bmad_dir, "bmb", "file.txt") + + # First run + removed1, not_found1, _ = cleanup_directories(bmad_dir, ["bmb"]) + self.assertEqual(removed1, ["bmb"]) + self.assertEqual(not_found1, []) + + # Second run — idempotent + removed2, not_found2, count2 = cleanup_directories(bmad_dir, ["bmb"]) + self.assertEqual(removed2, []) + self.assertEqual(not_found2, ["bmb"]) + self.assertEqual(count2, 0) + + +class TestSafetyCheck(unittest.TestCase): + def test_no_skills_dir_skips_check(self): + """When --skills-dir is not provided, no verification happens.""" + with tempfile.TemporaryDirectory() as tmpdir: + bmad_dir = os.path.join(tmpdir, "_bmad") + _make_skill_dir(bmad_dir, "bmb", "skills", "some-skill") + + # No skills_dir — cleanup should proceed without verification + removed, not_found, count = cleanup_directories(bmad_dir, ["bmb"]) + self.assertEqual(removed, ["bmb"]) + + def test_missing_skill_blocks_removal(self): + """When --skills-dir is provided and a skill is missing, exit 1.""" + with tempfile.TemporaryDirectory() as tmpdir: + bmad_dir = os.path.join(tmpdir, "_bmad") + skills_dir = os.path.join(tmpdir, "skills") + + _make_skill_dir(bmad_dir, "bmb", "skills", "installed-skill") + _make_skill_dir(bmad_dir, "bmb", "skills", "missing-skill") + + os.makedirs(os.path.join(skills_dir, "installed-skill")) + # missing-skill not created in skills_dir + + with self.assertRaises(SystemExit) as ctx: + verify_skills_installed(bmad_dir, ["bmb"], skills_dir) + self.assertEqual(ctx.exception.code, 1) + + # Directory should NOT have been removed (verification failed before cleanup) + self.assertTrue(os.path.isdir(os.path.join(bmad_dir, "bmb"))) + + def test_dir_without_skills_not_checked(self): + """Directories like _config that have no SKILL.md pass verification.""" + with tempfile.TemporaryDirectory() as tmpdir: + bmad_dir = os.path.join(tmpdir, "_bmad") + skills_dir = os.path.join(tmpdir, "skills") + + _make_file(bmad_dir, "_config", "manifest.yaml") + os.makedirs(skills_dir) + + # Should not raise — _config has no skills to verify + result = verify_skills_installed(bmad_dir, ["_config"], skills_dir) + self.assertEqual(result, []) + + +class TestEndToEnd(unittest.TestCase): + def test_full_cleanup_with_verification(self): + """Simulate complete cleanup flow with safety check.""" + with tempfile.TemporaryDirectory() as tmpdir: + bmad_dir = os.path.join(tmpdir, "_bmad") + skills_dir = os.path.join(tmpdir, "skills") + + # Create legacy structure + _make_skill_dir(bmad_dir, "bmb", "skills", "bmad-agent-builder") + _make_skill_dir(bmad_dir, "bmb", "skills", "bmad-builder-setup") + _make_file(bmad_dir, "bmb", "skills", "bmad-agent-builder", "assets", "template.md") + _make_skill_dir(bmad_dir, "core", "skills", "bmad-brainstorming") + _make_file(bmad_dir, "_config", "manifest.yaml") + _make_file(bmad_dir, "_config", "bmad-help.csv") + + # Create root config files that must survive + _make_file(bmad_dir, "config.yaml", content="document_output_language: English") + _make_file(bmad_dir, "config.user.yaml", content="user_name: Test") + _make_file(bmad_dir, "module-help.csv", content="module,name\nbmb,builder") + + # Create other module dirs that must survive + _make_file(bmad_dir, "bmm", "config.yaml") + _make_file(bmad_dir, "tea", "config.yaml") + + # Create installed skills + os.makedirs(os.path.join(skills_dir, "bmad-agent-builder")) + os.makedirs(os.path.join(skills_dir, "bmad-builder-setup")) + os.makedirs(os.path.join(skills_dir, "bmad-brainstorming")) + + # Verify + verified = verify_skills_installed( + bmad_dir, ["bmb", "core", "_config"], skills_dir + ) + self.assertIn("bmad-agent-builder", verified) + self.assertIn("bmad-builder-setup", verified) + self.assertIn("bmad-brainstorming", verified) + + # Cleanup + removed, not_found, file_count = cleanup_directories( + bmad_dir, ["bmb", "core", "_config"] + ) + self.assertEqual(sorted(removed), ["_config", "bmb", "core"]) + self.assertEqual(not_found, []) + self.assertGreater(file_count, 0) + + # Verify final state + self.assertFalse(os.path.exists(os.path.join(bmad_dir, "bmb"))) + self.assertFalse(os.path.exists(os.path.join(bmad_dir, "core"))) + self.assertFalse(os.path.exists(os.path.join(bmad_dir, "_config"))) + + # Root config files survived + self.assertTrue(os.path.exists(os.path.join(bmad_dir, "config.yaml"))) + self.assertTrue(os.path.exists(os.path.join(bmad_dir, "config.user.yaml"))) + self.assertTrue(os.path.exists(os.path.join(bmad_dir, "module-help.csv"))) + + # Other modules survived + self.assertTrue(os.path.isdir(os.path.join(bmad_dir, "bmm"))) + self.assertTrue(os.path.isdir(os.path.join(bmad_dir, "tea"))) + + def test_simulate_post_merge_scripts(self): + """Simulate the full flow: merge scripts run first (delete config files), + then cleanup removes directories.""" + with tempfile.TemporaryDirectory() as tmpdir: + bmad_dir = os.path.join(tmpdir, "_bmad") + + # Legacy state: config files already deleted by merge scripts + # but directories and skill content remain + _make_skill_dir(bmad_dir, "bmb", "skills", "bmad-agent-builder") + _make_file(bmad_dir, "bmb", "skills", "bmad-agent-builder", "refs", "doc.md") + _make_file(bmad_dir, "bmb", ".DS_Store") + # config.yaml already deleted by merge-config.py + # module-help.csv already deleted by merge-help-csv.py + + _make_skill_dir(bmad_dir, "core", "skills", "bmad-help") + # core/config.yaml already deleted + # core/module-help.csv already deleted + + # Root files from merge scripts + _make_file(bmad_dir, "config.yaml", content="bmb:\n name: BMad Builder") + _make_file(bmad_dir, "config.user.yaml", content="user_name: Test") + _make_file(bmad_dir, "module-help.csv", content="module,name") + + # Cleanup directories + removed, not_found, file_count = cleanup_directories( + bmad_dir, ["bmb", "core"] + ) + self.assertEqual(sorted(removed), ["bmb", "core"]) + self.assertGreater(file_count, 0) + + # Final state: only root config files + remaining = os.listdir(bmad_dir) + self.assertEqual( + sorted(remaining), + ["config.user.yaml", "config.yaml", "module-help.csv"], + ) + + +if __name__ == "__main__": + unittest.main() diff --git a/src/skills/bmad-builder-setup/scripts/tests/test-merge-config.py b/src/skills/bmad-builder-setup/scripts/tests/test-merge-config.py index b76ca81..179b163 100644 --- a/src/skills/bmad-builder-setup/scripts/tests/test-merge-config.py +++ b/src/skills/bmad-builder-setup/scripts/tests/test-merge-config.py @@ -33,6 +33,7 @@ load_legacy_values = merge_config_mod.load_legacy_values apply_legacy_defaults = merge_config_mod.apply_legacy_defaults cleanup_legacy_configs = merge_config_mod.cleanup_legacy_configs +apply_result_templates = merge_config_mod.apply_result_templates SAMPLE_MODULE_YAML = { @@ -139,6 +140,48 @@ def test_empty_answers(self): self.assertEqual(result, {}) +class TestApplyResultTemplates(unittest.TestCase): + def test_applies_template(self): + answers = {"bmad_builder_output_folder": "skills"} + result = apply_result_templates(SAMPLE_MODULE_YAML, answers) + self.assertEqual(result["bmad_builder_output_folder"], "{project-root}/skills") + + def test_applies_multiple_templates(self): + answers = { + "bmad_builder_output_folder": "skills", + "bmad_builder_reports": "skills/reports", + } + result = apply_result_templates(SAMPLE_MODULE_YAML, answers) + self.assertEqual(result["bmad_builder_output_folder"], "{project-root}/skills") + self.assertEqual(result["bmad_builder_reports"], "{project-root}/skills/reports") + + def test_skips_when_no_template(self): + """Variables without a result field are stored as-is.""" + yaml_no_result = { + "code": "test", + "my_var": {"prompt": "Enter value", "default": "foo"}, + } + answers = {"my_var": "bar"} + result = apply_result_templates(yaml_no_result, answers) + self.assertEqual(result["my_var"], "bar") + + def test_skips_when_value_already_has_project_root(self): + """Prevent double-prefixing if value already contains {project-root}.""" + answers = {"bmad_builder_output_folder": "{project-root}/skills"} + result = apply_result_templates(SAMPLE_MODULE_YAML, answers) + self.assertEqual(result["bmad_builder_output_folder"], "{project-root}/skills") + + def test_empty_answers(self): + result = apply_result_templates(SAMPLE_MODULE_YAML, {}) + self.assertEqual(result, {}) + + def test_unknown_key_passed_through(self): + """Keys not in module.yaml are passed through unchanged.""" + answers = {"unknown_key": "some_value"} + result = apply_result_templates(SAMPLE_MODULE_YAML, answers) + self.assertEqual(result["unknown_key"], "some_value") + + class TestMergeConfig(unittest.TestCase): def test_fresh_install_with_core_and_module(self): answers = { @@ -161,7 +204,7 @@ def test_fresh_install_with_core_and_module(self): self.assertEqual(result["document_output_language"], "English") self.assertEqual(result["output_folder"], "_bmad-output") self.assertEqual(result["bmb"]["name"], "BMad Builder") - self.assertEqual(result["bmb"]["bmad_builder_output_folder"], "_bmad-output/skills") + self.assertEqual(result["bmb"]["bmad_builder_output_folder"], "{project-root}/_bmad-output/skills") def test_update_strips_user_keys_preserves_shared(self): existing = { @@ -206,7 +249,7 @@ def test_anti_zombie_removes_existing_module(self): # Old variable is gone self.assertNotIn("old_variable", result["bmb"]) # New value is present - self.assertEqual(result["bmb"]["bmad_builder_output_folder"], "new/path") + self.assertEqual(result["bmb"]["bmad_builder_output_folder"], "{project-root}/new/path") # Metadata is fresh from module.yaml self.assertEqual(result["bmb"]["name"], "BMad Builder") @@ -329,7 +372,7 @@ def test_write_and_read_round_trip(self): # Shared core keys written self.assertEqual(written["document_output_language"], "English") self.assertEqual(written["output_folder"], "_bmad-output") - self.assertEqual(written["bmb"]["bmad_builder_output_folder"], "_bmad-output/skills") + self.assertEqual(written["bmb"]["bmad_builder_output_folder"], "{project-root}/_bmad-output/skills") def test_update_round_trip(self): """Simulate install, then re-install with different values.""" @@ -358,7 +401,7 @@ def test_update_round_trip(self): self.assertEqual(final["output_folder"], "/out") self.assertNotIn("user_name", final) - self.assertEqual(final["bmb"]["bmad_builder_output_folder"], "new/path") + self.assertEqual(final["bmb"]["bmad_builder_output_folder"], "{project-root}/new/path") class TestLoadLegacyValues(unittest.TestCase): @@ -593,8 +636,8 @@ def test_full_legacy_migration(self): # Shared core keys present self.assertEqual(final["document_output_language"], "French") self.assertEqual(final["output_folder"], "/legacy/out") - self.assertEqual(final["bmb"]["bmad_builder_output_folder"], "new/skills") - self.assertEqual(final["bmb"]["bmad_builder_reports"], "legacy/reports") + self.assertEqual(final["bmb"]["bmad_builder_output_folder"], "{project-root}/new/skills") + self.assertEqual(final["bmb"]["bmad_builder_reports"], "{project-root}/legacy/reports") if __name__ == "__main__":