Skip to content

Commit 9d2b886

Browse files
Sallvainianclaude
andauthored
fix: complete setup skill with legacy cleanup, result templates, and path fixes (#36)
* Add legacy directory cleanup and result template fix for setup skill - New cleanup-legacy.py script removes redundant installer directories from _bmad/ after config migration, with safety verification that skills exist at .claude/skills/ before removal - Fix merge-config.py to apply result templates from module.yaml so values like "skills" are stored as "{project-root}/skills" - Update SKILL.md with fresh-vs-legacy detection, directory creation step, and cleanup step - Add comprehensive tests for both changes (81 total, all passing) - Update module-configuration.md docs with cleanup section Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Remove inconsistent question tool hint and bundle language questions - Remove "Use a structured question tool if available" from Collect Configuration — causes inconsistent behavior across LLM sessions - Bundle communication_language and document_output_language into a single question since the answer is always the same Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Fix bare skill-internal paths to use ./ prefix All references to assets/ and scripts/ paths now use ./ prefix to distinguish them from {project-root} paths per BMad path conventions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: clarify {project-root} resolution is for filesystem ops only Address review feedback: the Overview says {project-root} is a literal token that should never be substituted, but Create Output Directories said to "resolve" it. Clarify that substitution applies only to filesystem operations (mkdir), not to stored config values. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 50eb6ac commit 9d2b886

6 files changed

Lines changed: 806 additions & 18 deletions

File tree

docs/explanation/module-configuration.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,12 @@ When the setup skill runs, it merges these rows into the project-wide `_bmad/mod
9898

9999
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.
100100

101+
## Legacy Directory Cleanup
102+
103+
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.
104+
105+
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.
106+
101107
## Design Guidance
102108

103109
Configuration is for **basic, project-level settings** — output folders, language preferences, feature toggles. Keep the number of configurable values small.

src/skills/bmad-builder-setup/SKILL.md

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ description: Sets up BMad Builder module in a project. Use when the user request
77

88
## Overview
99

10-
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:
10+
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:
1111

1212
- **`{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.
13-
- **`{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.
13+
- **`{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.
1414
- **`{project-root}/_bmad/module-help.csv`** — registers module capabilities for the help system.
1515

1616
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
2121

2222
1. Read `./assets/module.yaml` for module metadata and variable definitions (the `code` field is the module identifier)
2323
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
24-
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
24+
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:
25+
- 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.
26+
- 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.
27+
- In both cases, per-module config files and directories will be cleaned up after setup.
2528

2629
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.
2730

2831
## Collect Configuration
2932

30-
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.
33+
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.
3134

32-
**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.
35+
**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.
3336

34-
**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.
37+
**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.
3538

36-
**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).
39+
**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).
3740

3841
## Write Files
3942

4043
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:
4144

4245
```bash
43-
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"
44-
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}
46+
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"
47+
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}
4548
```
4649

4750
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.
4851

49-
Run `./scripts/merge-config.py --help` or `scripts/merge-help-csv.py --help` for full usage.
52+
Run `./scripts/merge-config.py --help` or `./scripts/merge-help-csv.py --help` for full usage.
53+
54+
## Create Output Directories
55+
56+
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.
57+
58+
## Cleanup Legacy Directories
59+
60+
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.
61+
62+
```bash
63+
python3 ./scripts/cleanup-legacy.py --bmad-dir "{project-root}/_bmad" --module-code {module-code} --also-remove _config --skills-dir "{project-root}/.claude/skills"
64+
```
65+
66+
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.
67+
68+
Check `directories_removed` and `files_removed_count` in the JSON output for the confirmation step. Run `./scripts/cleanup-legacy.py --help` for full usage.
5069

5170
## Confirm
5271

53-
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.
72+
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.
5473

5574
## Outcome
5675

Lines changed: 259 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,259 @@
1+
#!/usr/bin/env python3
2+
# /// script
3+
# requires-python = ">=3.9"
4+
# dependencies = []
5+
# ///
6+
"""Remove legacy module directories from _bmad/ after config migration.
7+
8+
After merge-config.py and merge-help-csv.py have migrated config data and
9+
deleted individual legacy files, this script removes the now-redundant
10+
directory trees. These directories contain skill files that are already
11+
installed at .claude/skills/ (or equivalent) — only the config files at
12+
_bmad/ root need to persist.
13+
14+
When --skills-dir is provided, the script verifies that every skill found
15+
in the legacy directories exists at the installed location before removing
16+
anything. Directories without skills (like _config/) are removed directly.
17+
18+
Exit codes: 0=success (including nothing to remove), 1=validation error, 2=runtime error
19+
"""
20+
21+
import argparse
22+
import json
23+
import shutil
24+
import sys
25+
from pathlib import Path
26+
27+
28+
def parse_args():
29+
parser = argparse.ArgumentParser(
30+
description="Remove legacy module directories from _bmad/ after config migration."
31+
)
32+
parser.add_argument(
33+
"--bmad-dir",
34+
required=True,
35+
help="Path to the _bmad/ directory",
36+
)
37+
parser.add_argument(
38+
"--module-code",
39+
required=True,
40+
help="Module code being cleaned up (e.g. 'bmb')",
41+
)
42+
parser.add_argument(
43+
"--also-remove",
44+
action="append",
45+
default=[],
46+
help="Additional directory names under _bmad/ to remove (repeatable)",
47+
)
48+
parser.add_argument(
49+
"--skills-dir",
50+
help="Path to .claude/skills/ — enables safety verification that skills "
51+
"are installed before removing legacy copies",
52+
)
53+
parser.add_argument(
54+
"--verbose",
55+
action="store_true",
56+
help="Print detailed progress to stderr",
57+
)
58+
return parser.parse_args()
59+
60+
61+
def find_skill_dirs(base_path: str) -> list:
62+
"""Find directories that contain a SKILL.md file.
63+
64+
Walks the directory tree and returns the leaf directory name for each
65+
directory containing a SKILL.md. These are considered skill directories.
66+
67+
Returns:
68+
List of skill directory names (e.g. ['bmad-agent-builder', 'bmad-builder-setup'])
69+
"""
70+
skills = []
71+
root = Path(base_path)
72+
if not root.exists():
73+
return skills
74+
for skill_md in root.rglob("SKILL.md"):
75+
skills.append(skill_md.parent.name)
76+
return sorted(set(skills))
77+
78+
79+
def verify_skills_installed(
80+
bmad_dir: str, dirs_to_check: list, skills_dir: str, verbose: bool = False
81+
) -> list:
82+
"""Verify that skills in legacy directories exist at the installed location.
83+
84+
Scans each directory in dirs_to_check for skill folders (containing SKILL.md),
85+
then checks that a matching directory exists under skills_dir. Directories
86+
that contain no skills (like _config/) are silently skipped.
87+
88+
Returns:
89+
List of verified skill names.
90+
91+
Raises SystemExit(1) if any skills are missing from skills_dir.
92+
"""
93+
all_verified = []
94+
missing = []
95+
96+
for dirname in dirs_to_check:
97+
legacy_path = Path(bmad_dir) / dirname
98+
if not legacy_path.exists():
99+
continue
100+
101+
skill_names = find_skill_dirs(str(legacy_path))
102+
if not skill_names:
103+
if verbose:
104+
print(
105+
f"No skills found in {dirname}/ — skipping verification",
106+
file=sys.stderr,
107+
)
108+
continue
109+
110+
for skill_name in skill_names:
111+
installed_path = Path(skills_dir) / skill_name
112+
if installed_path.is_dir():
113+
all_verified.append(skill_name)
114+
if verbose:
115+
print(
116+
f"Verified: {skill_name} exists at {installed_path}",
117+
file=sys.stderr,
118+
)
119+
else:
120+
missing.append(skill_name)
121+
if verbose:
122+
print(
123+
f"MISSING: {skill_name} not found at {installed_path}",
124+
file=sys.stderr,
125+
)
126+
127+
if missing:
128+
error_result = {
129+
"status": "error",
130+
"error": "Skills not found at installed location",
131+
"missing_skills": missing,
132+
"skills_dir": str(Path(skills_dir).resolve()),
133+
}
134+
print(json.dumps(error_result, indent=2))
135+
sys.exit(1)
136+
137+
return sorted(set(all_verified))
138+
139+
140+
def count_files(path: Path) -> int:
141+
"""Count all files recursively in a directory."""
142+
count = 0
143+
for item in path.rglob("*"):
144+
if item.is_file():
145+
count += 1
146+
return count
147+
148+
149+
def cleanup_directories(
150+
bmad_dir: str, dirs_to_remove: list, verbose: bool = False
151+
) -> tuple:
152+
"""Remove specified directories under bmad_dir.
153+
154+
Returns:
155+
(removed, not_found, total_files_removed) tuple
156+
"""
157+
removed = []
158+
not_found = []
159+
total_files = 0
160+
161+
for dirname in dirs_to_remove:
162+
target = Path(bmad_dir) / dirname
163+
if not target.exists():
164+
not_found.append(dirname)
165+
if verbose:
166+
print(f"Not found (skipping): {target}", file=sys.stderr)
167+
continue
168+
169+
if not target.is_dir():
170+
if verbose:
171+
print(f"Not a directory (skipping): {target}", file=sys.stderr)
172+
not_found.append(dirname)
173+
continue
174+
175+
file_count = count_files(target)
176+
if verbose:
177+
print(
178+
f"Removing {target} ({file_count} files)",
179+
file=sys.stderr,
180+
)
181+
182+
try:
183+
shutil.rmtree(target)
184+
except OSError as e:
185+
error_result = {
186+
"status": "error",
187+
"error": f"Failed to remove {target}: {e}",
188+
"directories_removed": removed,
189+
"directories_failed": dirname,
190+
}
191+
print(json.dumps(error_result, indent=2))
192+
sys.exit(2)
193+
194+
removed.append(dirname)
195+
total_files += file_count
196+
197+
return removed, not_found, total_files
198+
199+
200+
def main():
201+
args = parse_args()
202+
203+
bmad_dir = args.bmad_dir
204+
module_code = args.module_code
205+
206+
# Build the list of directories to remove
207+
dirs_to_remove = [module_code, "core"] + args.also_remove
208+
# Deduplicate while preserving order
209+
seen = set()
210+
unique_dirs = []
211+
for d in dirs_to_remove:
212+
if d not in seen:
213+
seen.add(d)
214+
unique_dirs.append(d)
215+
dirs_to_remove = unique_dirs
216+
217+
if args.verbose:
218+
print(f"Directories to remove: {dirs_to_remove}", file=sys.stderr)
219+
220+
# Safety check: verify skills are installed before removing
221+
verified_skills = None
222+
if args.skills_dir:
223+
if args.verbose:
224+
print(
225+
f"Verifying skills installed at {args.skills_dir}",
226+
file=sys.stderr,
227+
)
228+
verified_skills = verify_skills_installed(
229+
bmad_dir, dirs_to_remove, args.skills_dir, args.verbose
230+
)
231+
232+
# Remove directories
233+
removed, not_found, total_files = cleanup_directories(
234+
bmad_dir, dirs_to_remove, args.verbose
235+
)
236+
237+
# Build result
238+
result = {
239+
"status": "success",
240+
"bmad_dir": str(Path(bmad_dir).resolve()),
241+
"directories_removed": removed,
242+
"directories_not_found": not_found,
243+
"files_removed_count": total_files,
244+
}
245+
246+
if args.skills_dir:
247+
result["safety_checks"] = {
248+
"skills_verified": True,
249+
"skills_dir": str(Path(args.skills_dir).resolve()),
250+
"verified_skills": verified_skills,
251+
}
252+
else:
253+
result["safety_checks"] = None
254+
255+
print(json.dumps(result, indent=2))
256+
257+
258+
if __name__ == "__main__":
259+
main()

0 commit comments

Comments
 (0)