-
-
Notifications
You must be signed in to change notification settings - Fork 29
fix: complete setup skill with legacy cleanup, result templates, and path fixes #36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
c1519b9
Add legacy directory cleanup and result template fix for setup skill
Sallvainian 492457c
Remove inconsistent question tool hint and bundle language questions
Sallvainian fc2e37a
Fix bare skill-internal paths to use ./ prefix
Sallvainian 0833542
fix: clarify {project-root} resolution is for filesystem ops only
Sallvainian File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
259 changes: 259 additions & 0 deletions
259
src/skills/bmad-builder-setup/scripts/cleanup-legacy.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
|
Comment on lines
+203
to
+207
|
||
| # 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() | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please wrap long Markdown lines to avoid lint failures.
Several updated lines are very long and may fail markdown lint in CI. Please hard-wrap these paragraphs/lists into shorter lines.
Suggested wrap style
As per coding guidelines: "
**/*.md: Markdown files must pass markdown linting as part of the test suite".Also applies to: 56-57, 72-72
🤖 Prompt for AI Agents