refactor(installer): discover skills by SKILL.md instead of manifest YAML#2082
Merged
refactor(installer): discover skills by SKILL.md instead of manifest YAML#2082
Conversation
…YAML Switch skill discovery gate from requiring bmad-skill-manifest.yaml with type: skill to detecting any directory with a valid SKILL.md (frontmatter name + description, name matches directory name). Delete 34 stub manifests that carried no data beyond type: skill. Agent manifests (9) are retained for persona metadata consumed by agent-manifest.csv.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (36)
💤 Files with no reviewable changes (35)
📝 WalkthroughWalkthroughThis PR removes Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🤖 Augment PR SummarySummary: Refactors the installer’s manifest generation so skills are discovered via Changes:
Technical Notes: Skill discovery now depends on 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| const skillMdPath = path.join(dir, skillFile); | ||
| const dirName = path.basename(dir); | ||
|
|
||
| const skillMeta = await this.parseSkillMd(skillMdPath, dir, dirName, debug); |
There was a problem hiding this comment.
If a directory contains a SKILL.md that exists but fails parseSkillMd validation, it won’t be added to skillClaimedDirs, so its workflow.md (and other files) can still be picked up by the workflow/task scanners and end up in the wrong manifest. Consider whether “SKILL.md present but invalid” should still block legacy scanning (or at least emit a non-debug warning) to avoid silently misclassifying broken skills as workflows.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| const manifest = await this.loadSkillManifest(dir); | ||
| if (this.hasNativeSkillManifest(manifest)) return true; | ||
| // Check for SKILL.md in this directory | ||
| if (entries.some((e) => !e.isDirectory() && e.name === 'SKILL.md')) return true; |
There was a problem hiding this comment.
_hasSkillMdRecursive() treats any SKILL.md presence as a module signal without validating frontmatter, so scanInstalledModules() can include “skill-only” modules even when no valid skills will be discovered. Consider whether module detection should align with the same validation criteria used by collectSkills() to avoid polluting manifest.yaml/CSV outputs with empty modules.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| * with name/description frontmatter. | ||
| * A directory is discovered as a skill when it contains a SKILL.md file with | ||
| * valid name/description frontmatter (name must match directory name). | ||
| * Manifest YAML is loaded only when present — for install_to_bmad and agent metadata. |
There was a problem hiding this comment.
PR description says “Remove all bmad-skill-manifest.yaml files”, but the implementation still supports (and tests still rely on) manifest YAML for things like install_to_bmad and agent/workflow metadata. Might be worth clarifying in the PR description/changelog which YAML manifests are truly removed vs still supported/required.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
This was referenced Mar 21, 2026
This was referenced Mar 31, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
bmad-skill-manifest.yamlfiles — the installer now discovers skills by the presence ofSKILL.mdTest plan
npm run quality— all tests pass🤖 Generated with Claude Code