refactor: all-is-skills - Convert BMAD to skills-based architecture#1834
Merged
bmadcode merged 18 commits intobmad-code-org:mainfrom Mar 7, 2026
Merged
refactor: all-is-skills - Convert BMAD to skills-based architecture#1834bmadcode merged 18 commits intobmad-code-org:mainfrom
bmadcode merged 18 commits intobmad-code-org:mainfrom
Conversation
Add bmad-skill-manifest.yaml sidecars to all 38 capabilities (tasks, agents, workflows) declaring canonicalId as the single source of truth for skill names. Update Claude Code and Codex installers to prefer canonicalId over path-derived names, with graceful fallback. - 24 manifest files covering 38 capabilities - New shared skill-manifest.js utility for manifest loading - resolveSkillName() in path-utils.js bridges manifest → installer - All command generators propagate canonicalId through CSV manifests - Drops bmm module prefix from all user-facing skill names Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…LL.md Refactor the config-driven installer to emit Agent Skills Open Standard format for Claude Code: directory-per-skill with SKILL.md entrypoint, unquoted YAML frontmatter, and full canonical names. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Delete the custom codex.js installer (441 lines) and route Codex through the config-driven pipeline via platform-codes.yaml. This fixes 7 task/tool descriptions that were generic due to bypassing manifests, and eliminates duplicate transformToSkillFormat code. Key changes: - Add codex entry to platform-codes.yaml with skill_format + legacy_targets - Remove codex from custom installer list in manager.js - Add installCustomAgentLauncher() to config-driven for custom agent support - Add detect() override for skill_format platforms (bmad-prefix check) - Set configDir from target_dir for base-class detect() compatibility Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
alexeyv
force-pushed
the
refactor/all-is-skills
branch
from
fe69bf4 to
23fbec9
Compare
Contributor
|
@coderabbitai review |
🤖 Augment PR SummarySummary: Refactors BMAD toward a skills-based architecture by introducing per-skill manifests and updating IDE installers to emit native “Agent Skills” directory layouts. Changes:
Technical Notes: Skill naming now prefers manifest 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThis PR registers agents, workflows, and tasks with canonical IDs and descriptions through new skill manifest files, refactors the CLI installer to support skill-format output (per-skill directories with SKILL.md files), removes legacy Codex CLI implementation in favor of config-driven handling, and updates platform configurations to enable new skill organization patterns. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
tools/cli/installers/lib/ide/_config-driven.js (2)
740-748:⚠️ Potential issue | 🟠 MajorDon't wipe custom agent launchers during routine cleanup.
cleanupTarget()removes everybmad*entry, which includes thebmad-custom-agent-*skills created byinstallCustomAgentLauncher(). A normal reinstall will therefore delete user-created custom agents and never recreate them.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/_config-driven.js` around lines 740 - 748, cleanupTarget() is currently deleting every entry that starts with "bmad", which removes user-created launchers like those created by installCustomAgentLauncher(); update the entries loop to exclude the custom agent pattern (e.g., skip entries that startWith "bmad-custom-agent-") so only intended "bmad" items are removed; specifically adjust the condition inside the for-loop that checks entry.startsWith('bmad') to also check !entry.startsWith('bmad-custom-agent-') (or a equivalent regex) before calling fs.remove on entryPath.
782-800:⚠️ Potential issue | 🟠 MajorStop ancestor-conflict scans at the platform's real discovery boundary.
This walks all the way to filesystem root. For OpenCode, the new tests and migration notes in this PR describe inheritance only up to the git worktree, so an unrelated
.opencode/skillshigher up the home directory will trigger a false conflict. Bound the scan to the platform's actual workspace root instead of/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/_config-driven.js` around lines 782 - 800, The ancestor scan currently climbs to the filesystem root (root) causing false conflicts; change the stopping boundary from root to the platform workspace root by resolving the Git worktree/top-level directory for resolvedProject and using that value (e.g., workspaceRoot) in place of root when walking current; implement a helper that runs git rev-parse --show-toplevel (or falls back to path.parse(resolvedProject).root) and use its result as the root boundary for the while loop and candidatePath checks (references: resolvedProject, current, root, candidatePath, targetDir).tools/cli/installers/lib/ide/shared/bmad-artifacts.js (1)
38-53:⚠️ Potential issue | 🟠 MajorUse the same agent filters for standalone agent directories.
This branch still treats any non-customize
.mdfile as an agent, whilegetAgentsFromDir()rejects README-style files and requires<agent>content. After adding manifest-derived canonical IDs, a stray README/SKILL doc in a standalone directory will be exported as a second agent sharing the same manifest ID.Possible fix
for (const file of agentFiles) { if (!file.endsWith('.md')) continue; + if (file.toLowerCase() === 'readme.md' || file.toLowerCase().startsWith('readme-')) continue; if (file.includes('.customize.')) continue; const filePath = path.join(agentDirPath, file); const content = await fs.readFile(filePath, 'utf8'); if (content.includes('localskip="true"')) continue; + if (!content.includes('<agent')) continue; agents.push({ path: filePath, name: file.replace('.md', ''), module: 'standalone', // Mark as standalone agent🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/shared/bmad-artifacts.js` around lines 38 - 53, The standalone-agent branch is including any non-customize `.md` (e.g., README/SKILL docs) and then assigning manifest-derived canonical IDs which can create duplicate agents; update this branch (the block using loadSkillManifest, getCanonicalId and agents.push) to apply the same filters used by getAgentsFromDir: skip README-style files (e.g., README.md / SKILL.md), require the file content to contain an `<agent>` token, and preserve the localskip check — additionally guard against duplicate canonical IDs by tracking seen IDs before pushing to agents so you don't export a second agent with the same manifest ID.tools/cli/installers/lib/core/manifest-generator.js (3)
829-844:⚠️ Potential issue | 🟠 MajorAgent merging still keys off the unstable filename-derived name.
Line 830 uses
module:name, so a rename with the samecanonicalIdleaves the old row behind, and directory-based skills become fragile if filenames stop being unique. UsecanonicalIdas the primary key and only fall back topath.Also applies to: 849-862
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/core/manifest-generator.js` around lines 829 - 844, The merge currently uses a filename-derived key `${agent.module}:${agent.name}` (key variable) to populate allAgents from this.agents, which breaks when names change; change the key logic to use agent.canonicalId as the primary unique identifier and fall back to agent.path when canonicalId is missing or empty, and apply the same canonicalId-first key logic to the other merging block that handles the secondary agent list (the block around the second merge currently at 849-862) so entries are deduplicated by canonicalId/path instead of module:name.
769-777:⚠️ Potential issue | 🟠 Major
module:nameis the wrong dedupe key for workflows now.Line 770 collapses same-name workflows inside a module, even though the collector deliberately admits multiple workflow file formats. Prefer
canonicalIdfirst and fall back topath, otherwise one row just disappears.Also applies to: 782-788
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/core/manifest-generator.js` around lines 769 - 777, The dedupe key for workflows uses `${workflow.module}:${workflow.name}` which collapses distinct formats; change the key generation in the workflow loop (the code creating `const key = ...` and calling `allWorkflows.set(key, {...})`) to use the canonical identifier first and fall back to path (e.g. `${workflow.module}:${workflow.canonicalId || workflow.path}`) so each distinct workflow file stays unique; apply the same fix to the second similar loop around lines 782-788 where a key is built and items are added (replace `${...:name}` fallback with `canonicalId || path`).
903-913: 🛠️ Refactor suggestion | 🟠 MajorTasks and tools have the same stale-row problem.
Lines 904 and 968 still treat
module:nameas the stable identity. Under the new manifest model, that should becanonicalId; otherwise renames and reorganizations create duplicate rows instead of updates.Also applies to: 918-926, 967-977, 982-990
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/core/manifest-generator.js` around lines 903 - 913, The code builds map keys from `${task.module}:${task.name}` which causes stale-row duplicates; change the key generation in the loops that iterate this.tasks and this.tools (where key is currently assigned) to use the stable canonicalId when present, e.g. use const key = task.canonicalId && task.canonicalId.length ? task.canonicalId : `${task.module}:${task.name}` (or equivalent) and apply the same pattern wherever you build keys for tasks, tools, and any task-tool join entries so lookups/updates use canonicalId first and fall back to module:name only if canonicalId is absent.
🧹 Nitpick comments (11)
src/bmm/workflows/document-project/bmad-skill-manifest.yaml (1)
3-3: Description uses jargon that limits discoverability."Brownfield projects" is industry jargon that may not be clear to all users. Additionally, if this workflow also works for greenfield projects, the description artificially limits its apparent scope.
Consider: "Document existing codebases to establish AI context" or similar.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/document-project/bmad-skill-manifest.yaml` at line 3, Update the manifest's description field to remove the jargon "brownfield projects" and use a clearer, broader phrase; edit the description value in bmad-skill-manifest.yaml (the description: "Document brownfield projects for AI context" entry) to something like "Document existing codebases to establish AI context" or similar wording that avoids industry-specific terms and does not exclude greenfield use cases.src/bmm/workflows/2-plan-workflows/create-ux-design/bmad-skill-manifest.yaml (1)
1-3: Missing schema version field for manifest evolution.These skill manifests have no version or schema field. When the manifest schema inevitably changes (e.g., adding
enabled,dependencies,tags), there's no way to distinguish v1 manifests from v2 during automated migration.Consider adding a
schemaVersion: 1field now to enable forward-compatible tooling.Proposed addition
+schemaVersion: 1 canonicalId: bmad-create-ux-design type: workflow description: "Plan UX patterns and design specifications"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/2-plan-workflows/create-ux-design/bmad-skill-manifest.yaml` around lines 1 - 3, The manifest for the workflow (canonicalId: bmad-create-ux-design, type: workflow, description: "Plan UX patterns and design specifications") is missing a schema/version marker; add a schemaVersion field (e.g., schemaVersion: 1) at the top of the YAML to allow future schema evolution and automated migration tools to distinguish v1 manifests from later versions; ensure any manifest-parsing code that reads canonicalId or type can fall back to schemaVersion when present.tools/cli/installers/lib/ide/shared/workflow-command-generator.js (1)
90-101: Artifact structure lacks JSDoc documenting thecanonicalIdfield.The artifact object structure now includes
canonicalId, but there's no JSDoc or type annotation describing it. Consumers ofcollectWorkflowArtifactswon't know what values to expect (string, optional, constraints).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/shared/workflow-command-generator.js` around lines 90 - 101, Add JSDoc/type info for the artifact object's canonicalId so consumers know it's an optional string and any constraints; update the typedef or JSDoc for the artifact shape (e.g., the WorkflowArtifact typedef or the JSDoc above collectWorkflowArtifacts) to include a line like "canonicalId?: string — optional canonical identifier for the workflow, if present must be a non-empty string" and ensure the documentation appears next to where artifacts.push(...) is defined (or the function collectWorkflowArtifacts) so tooling and IDEs pick up the field description and type.test/test-installation-components.js (2)
202-223: Move temp-project cleanup intofinally.This pattern only removes the temp project on the success path. The first failed assertion leaks directories under the system temp root, and the same structure repeats in the later platform blocks. Wrap each
mkdtemp()block intry/finallyso cleanup always runs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/test-installation-components.js` around lines 202 - 223, The temp project created by fs.mkdtemp (tempProjectDir) is only removed on the success path; wrap the block that uses tempProjectDir (from the fs.mkdtemp call through all assertions and IdeManager.setup/result checks) in a try/finally and move the cleanup call (await fs.remove(tempProjectDir)) into the finally so the temp directory is removed even on test failures; apply the same try/finally pattern to any other repeated mkdtemp blocks in this file to ensure no temp dirs are leaked.
218-219: These migration tests still miss most of the changed surface.Each new native-skills scenario only proves that one
bmad-master/SKILL.mddirectory exists. Because workflows and tasks are installed globally regardless ofselectedModules, that assertion says nothing about agent skills, task/tool skills, or canonical-id naming. Add at least one agent assertion and one task/tool assertion per platform.Based on learnings: "workflows and tasks are cross-cutting methodology artifacts that should always be installed globally, regardless of which modules are selected. Only agents are module-scoped and should be filtered by selectedModules."
Also applies to: 265-266, 312-313, 364-365, 424-425
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/test-installation-components.js` around lines 218 - 219, The test only asserts the existence of a global SKILL.md (skillFile) and misses verifying module-scoped agents and globally-installed tasks/tools; update the tests that create skillFile (using tempProjectDir and path.join('.windsurf','skills',...,'SKILL.md')) to also assert at least one agent-specific skill file (e.g., under the agent module name expected when selectedModules filters apply) and one task/tool skill file that should be present globally; apply the same pattern to the other occurrences referenced (the checks around lines 265-266, 312-313, 364-365, 424-425) so each platform test includes one agent-scoped assertion and one task/tool/global assertion, and ensure the agent assertions reflect selectedModules scoping while task/tool assertions validate global installation.src/bmm/workflows/4-implementation/create-story/bmad-skill-manifest.yaml (1)
1-3: Minor: Description verb tense inconsistency.Other manifests use imperative mood ("Implement...", "Break...", "Manage...") while this uses indicative ("Creates..."). Consider "Create a dedicated story file..." for consistency across the manifest collection.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/create-story/bmad-skill-manifest.yaml` around lines 1 - 3, Update the description field for the workflow with canonicalId bmad-create-story to use imperative mood for consistency; change the string value of the description key from "Creates a dedicated story file with all the context needed for implementation" to "Create a dedicated story file with all the context needed for implementation" so it matches other manifest descriptions.src/bmm/workflows/bmad-quick-flow/quick-dev/bmad-skill-manifest.yaml (1)
1-3: Description semantics may be misleading.The description says "Implement a Quick Tech Spec" which could be interpreted as "create a spec." Given the sibling
quick-specworkflow explicitly handles spec creation, consider rewording to clarify this workflow implements code based on a spec, e.g., "Implement code changes from a Quick Tech Spec."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/bmad-quick-flow/quick-dev/bmad-skill-manifest.yaml` around lines 1 - 3, Update the manifest's description to avoid implying spec creation: change the description field (canonicalId: bmad-quick-dev) from "Implement a Quick Tech Spec for small changes or features" to a clearer phrase such as "Implement code changes from a Quick Tech Spec for small changes or features" so it's obvious this workflow implements code based on an existing quick-spec workflow rather than creating the spec.src/bmm/workflows/bmad-quick-flow/quick-spec/bmad-skill-manifest.yaml (1)
3-3: Description is verbose with redundancy."Very quick process to create implementation-ready quick specs" uses "quick" twice and "Very quick process" adds little value. Consider a more concise description.
Suggested fix
canonicalId: bmad-quick-spec type: workflow -description: "Very quick process to create implementation-ready quick specs for small changes or features" +description: "Create implementation-ready specs for small changes or features"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/bmad-quick-flow/quick-spec/bmad-skill-manifest.yaml` at line 3, The description field in the bmad-skill-manifest.yaml is redundant and verbose; replace the current value ("Very quick process to create implementation-ready quick specs for small changes or features") with a concise, non-redundant phrase such as "Create implementation-ready quick specs for small changes" by editing the description property in the manifest (the description: string entry).src/bmm/workflows/4-implementation/code-review/bmad-skill-manifest.yaml (1)
1-3: Version the manifest format before this spreads further.This is a brand-new public contract, but there is no
schemaVersion/manifestVersionfield. That leaves loaders no clean way to distinguish future format changes from malformed files once dozens of these are in the tree.Possible direction
+schemaVersion: 1 canonicalId: bmad-code-review type: workflow description: "Perform adversarial code review finding specific issues"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bmm/workflows/4-implementation/code-review/bmad-skill-manifest.yaml` around lines 1 - 3, Add a top-level manifestVersion (or schemaVersion) field to this manifest (e.g., manifestVersion: "1.0") so consumers can detect format changes; update the manifest that contains canonicalId: bmad-code-review and type: workflow to include that field, document that it should follow semantic versioning and be bumped on breaking changes, and ensure any loader/validator that parses manifests checks manifestVersion before interpreting fields.tools/cli/installers/lib/ide/shared/bmad-artifacts.js (1)
38-38: Cache manifest loads instead of reparsing the same YAML on every traversal.With sidecar manifests now added across the tree,
loadSkillManifest()becomes repeated disk I/O during agent/task collection and across multiple IDE installs. A simple per-directory cache would keep this from becoming a death-by-a-thousand-cuts hot path.Also applies to: 90-90, 147-147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/shared/bmad-artifacts.js` at line 38, loadSkillManifest is being called repeatedly causing redundant disk I/O; add a simple per-directory in-memory cache (e.g., a Map named skillManifestCache keyed by agentDirPath) and update all call sites that currently call loadSkillManifest (including the usages that mirror the lines noted) to first check skillManifestCache.get(agentDirPath) and return the cached manifest if present, otherwise call loadSkillManifest, store the result in skillManifestCache.set(agentDirPath, manifest) and then return it; ensure the cache is module-scoped so loadSkillManifest callers across this file reuse it and consider optionally invalidating entries if manifest mtime checking is desired.tools/cli/installers/lib/ide/manager.js (1)
61-65: Stop hand-maintaining the custom-installer allowlist.The file header talks about dynamic discovery, but Line 65 is just a hardcoded array. That means the next custom installer can land in this directory and be silently ignored until someone remembers to update this list. Prefer directory discovery with an explicit denylist, or move the registry to one authoritative config file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/manager.js` around lines 61 - 65, The hardcoded allowlist in loadCustomInstallerFiles (variable customFiles) prevents new installer files in __dirname from being discovered; change to dynamic directory discovery: read the directory contents (fs.readdir or readdirSync) under __dirname, filter for installer JS files (e.g., /\.js$/) and exclude a small explicit denylist (e.g., README, index, or known non-installer filenames) instead of maintaining customFiles; update loadCustomInstallerFiles to build the final list from the discovered files so new installers are auto-picked up.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/bmm/workflows/3-solutioning/create-architecture/bmad-skill-manifest.yaml`:
- Line 3: Update the skill manifest's description key in
bmad-skill-manifest.yaml to clearly state the artifact produced rather than the
rationale: replace the current value of the description field with a concise,
user-facing sentence that names the output (e.g., "Creates architecture solution
design decisions and an architecture specification for AI agents" or "Generates
architecture/design decisions and an architecture spec"); ensure the new text
appears under the description key so users browsing installed skills immediately
see what artifact this workflow produces.
In
`@src/bmm/workflows/3-solutioning/create-epics-and-stories/bmad-skill-manifest.yaml`:
- Around line 1-3: Add a schemaVersion field and validate manifests in
skill-manifest.js: update all manifest files (e.g., the YAML shown containing
canonicalId, type, description) to include schemaVersion: 1, then enhance
skill-manifest.js to parse and validate manifests against a simple schema that
requires canonicalId (string), type (enum: "workflow" or "agent"), description
(string), and schemaVersion (integer and equals 1); implement explicit error
messages for missing/invalid fields and export or document the schema
(schemaVersion 1) centrally so future versions can be added and validated.
In `@src/bmm/workflows/4-implementation/code-review/bmad-skill-manifest.yaml`:
- Line 3: Update the manifest "description" field to clearly state the
workflow's actual output and approach: replace the generic "Perform adversarial
code review finding specific issues" with a concise description that mentions it
performs an adversarial, line-by-line code review and returns severity-tagged
findings (e.g., critical/high/medium/low) and suggested remediations; locate the
"description" key in the bmad-skill-manifest.yaml and rewrite the string to
reflect the adversarial review method and the severity-tagged findings returned.
In `@src/bmm/workflows/4-implementation/dev-story/bmad-skill-manifest.yaml`:
- Line 3: The manifest's description field contains internal jargon
("context-filled story spec file"); update the description key in
bmad-skill-manifest.yaml to a user-facing phrase that describes the skill's
behavior (e.g., "Implement an approved story in code and tests from a provided
story specification"), avoiding internal terms like "context-filled" or "spec
file"; ensure the new description succinctly states the user-visible action
("implement a story in code/tests from an approved story spec") so consumers of
the manifest understand what the skill does.
In `@src/bmm/workflows/4-implementation/sprint-planning/bmad-skill-manifest.yaml`:
- Line 3: The manifest description in the bmad skill manifest's description
field is incorrect for the sprint-planning workflow; update the description
string in bmad-skill-manifest.yaml (the description field for the
sprint-planning workflow) to accurately describe sprint planning functionality
(for example: "Plan and organize sprint work from epics" or similar) so it no
longer duplicates the sprint-status text and avoids user confusion when
selecting workflows.
In
`@src/bmm/workflows/bmad-quick-flow/quick-dev-new-preview/bmad-skill-manifest.yaml`:
- Around line 1-3: The manifest's canonicalId embeds a temporary maturity label
("bmad-quick-dev-new-preview"), which will become stale; change the canonicalId
to a stable identifier (e.g., use "bmad-quick-unified-flow" or similar) and add
a separate manifest field to express maturity (e.g., add a new top-level key
like status: preview) so you avoid baking lifecycle state into canonicalId;
update any references to the old canonicalId to point to the new stable id and
ensure consumers read the new status field for preview/graduation checks.
In `@src/bmm/workflows/generate-project-context/bmad-skill-manifest.yaml`:
- Line 3: Update the ambiguous description in bmad-skill-manifest.yaml: replace
"Create project-context.md with AI rules" with a clear, user-facing phrase that
explains what "AI rules" means (e.g., "Create project-context.md with coding
guidelines and project constraints for AI assistants" or similar). Edit the
description field in the manifest so it explicitly states the output and its
purpose (coding guidelines, context constraints, or other specified rules) to
remove ambiguity for downstream users.
- Around line 1-3: Add a validation step that scans all bmad-skill-manifest.yaml
files for duplicate canonicalId values and fails the build/pre-commit if any
duplicates are found; implement this by extending or adding a script (e.g.,
update validate-agent-schema.js or create a new validate-canonical-ids.js) to
load each manifest, collect the canonicalId field, detect duplicates, and return
a non-zero exit code with a clear error message referencing the duplicated
canonicalId(s) so CI/pre-commit hooks block the commit.
In `@src/core/workflows/party-mode/bmad-skill-manifest.yaml`:
- Around line 1-3: Update the description field for canonicalId
"bmad-party-mode" to be specific about the workflow's capability and intended
use: replace the vague "Orchestrates group discussions between all installed
BMAD agents" with a short sentence that states what the workflow does (e.g.,
facilitates multi-agent brainstorming, consensus-building, or task
coordination), when to invoke it (e.g., to generate ideas, resolve
disagreements, or synthesize agent outputs), and what the user can expect as
output (e.g., consolidated summary, ranked suggestions, or agreed action plan);
keep it concise and user-facing so someone scanning the manifest understands the
value proposition and expected result.
In `@tools/cli/installers/lib/core/manifest-generator.js`:
- Line 843: The manifest generator currently unconditionally sets canonicalId to
agent.canonicalId || '' which can overwrite a previously valid value with an
empty string; change the logic so you only write the canonicalId field when
agent.canonicalId is non-empty (e.g., check agent.canonicalId is
truthy/non-empty) and otherwise leave the existing canonicalId untouched (or
omit the field), and apply the same fix for the other occurrences that use this
pattern (the canonicalId assignments at the other noted spots).
- Line 237: Before serializing manifests, add a validation pass that collects
every canonicalId produced by getCanonicalId(...) (the values assigned to
canonicalId for each entry.name/skillManifest), then reject and surface errors
for any empty/null values or duplicate canonicalIds; when reporting an error
include the owning artifact identifier/path (use the existing skillManifest or
entry metadata fields that hold the artifact path) and stop generation before
writing the manifest. Implement this check in the manifest generation flow right
after canonicalId assignment points (where canonicalId is set from
getCanonicalId) so you validate all collected IDs and throw or return a clear
error listing missing or duplicated IDs and their corresponding
skillManifest/entry owner.
In `@tools/cli/installers/lib/ide/_config-driven.js`:
- Around line 509-518: In transformToSkillFormat, the no-frontmatter fallback
currently sets description to skillName but must use the same default as the
frontmatter branch; change the generated frontmatter (variable fm) to set
description:`${skillName} skill` instead of `skillName`, and ensure any empty
description values are normalized to `${skillName} skill` so the Agent Skills
"description" field is never empty.
- Around line 481-499: In writeSkillFile, before creating/writing
skillDir/SKILL.md, detect collisions by checking if skillDir already exists (use
fs.stat/exists or this.ensureDir semantics) and if so read the existing SKILL.md
(or a dedicated metadata marker) to extract the originating artifact
identifier/canonicalId (parse frontmatter or a stored JSON marker); if the
existing canonicalId differs from artifact.canonicalId, throw a clear Error
naming both artifacts (existing and incoming) and the colliding canonicalId
instead of overwriting; otherwise proceed to call transformToSkillFormat and
writeFile as before. References: writeSkillFile, resolveSkillName, skillDir,
SKILL.md, transformToSkillFormat, writeFile, artifact.canonicalId.
In `@tools/cli/installers/lib/ide/shared/path-utils.js`:
- Around line 275-279: validate artifact.canonicalId in resolveSkillName before
using it as a filename: ensure it's a string and matches a tight safe regex (no
slashes, dots, or traversal, and within your expected namespace, e.g. starts
with the bmad- prefix and only contains lowercase letters, numbers and hyphens)
and only then return `${artifact.canonicalId}.md`; if it fails validation, fall
back to the existing toDashPath(artifact.relativePath) (or otherwise sanitize
the value) to avoid directory escape, nested paths, or bypassing bmad-*
detection; update resolveSkillName to perform this check on artifact.canonicalId
and use toDashPath as the safe alternative.
In `@tools/cli/installers/lib/ide/shared/skill-manifest.js`:
- Around line 19-20: The current single-entry detection flips to __single
whenever any parsed.canonicalId exists; tighten it by requiring an exclusive
single-entry shape: in the conditional that now checks parsed.canonicalId (the
block that returns { __single: parsed }), change it to verify parsed is an
object and that Object.keys(parsed).length === 1 (only the canonicalId key) and
that parsed.canonicalId is truthy before returning { __single: parsed };
otherwise return parsed as before.
- Around line 17-18: The current check returns any object from
yaml.parse(content) (see parsed) which allows arrays or malformed sidecar
entries; instead validate the manifest shape before returning: ensure parsed is
a plain object (not an array), contains the expected sidecar/key properties, and
that each sidecar entry has a usable canonicalId and the expected type value
(reject entries missing canonicalId or with mismatched type). Update the check
around parsed in skill-manifest.js to perform these schema validations and
return null on failure so downstream code never receives malformed manifests.
- Around line 21-23: The catch block that currently does console.warn(...) and
returns null should instead surface the failure so the installer fails fast:
replace the silent return with throwing an Error (or rethrowing the caught
error) that includes the manifest filename ("bmad-skill-manifest.yaml") and the
dirPath to make the failure explicit; update the catch in the function that
parses/loads the manifest (the catch where dirPath and error.message are
referenced) to throw a new Error with a clear message combining the manifest
path and original error details rather than returning null.
In `@tools/cli/installers/lib/ide/shared/workflow-command-generator.js`:
- Around line 94-96: The fallback for workflow.canonicalId currently uses an
empty string which can silently break downstream consumers; update the object
construction that sets canonicalId so it either derives a meaningful fallback
from workflow.name (e.g., canonicalId: workflow.canonicalId || workflow.name) or
emits a visible warning when missing using the existing prompts.log.warn helper;
locate the code that sets description/module/canonicalId and replace the
empty-string fallback with the derived value or add a prompts.log.warn call when
workflow.canonicalId is falsy to surface the manifest gap during install.
---
Outside diff comments:
In `@tools/cli/installers/lib/core/manifest-generator.js`:
- Around line 829-844: The merge currently uses a filename-derived key
`${agent.module}:${agent.name}` (key variable) to populate allAgents from
this.agents, which breaks when names change; change the key logic to use
agent.canonicalId as the primary unique identifier and fall back to agent.path
when canonicalId is missing or empty, and apply the same canonicalId-first key
logic to the other merging block that handles the secondary agent list (the
block around the second merge currently at 849-862) so entries are deduplicated
by canonicalId/path instead of module:name.
- Around line 769-777: The dedupe key for workflows uses
`${workflow.module}:${workflow.name}` which collapses distinct formats; change
the key generation in the workflow loop (the code creating `const key = ...` and
calling `allWorkflows.set(key, {...})`) to use the canonical identifier first
and fall back to path (e.g. `${workflow.module}:${workflow.canonicalId ||
workflow.path}`) so each distinct workflow file stays unique; apply the same fix
to the second similar loop around lines 782-788 where a key is built and items
are added (replace `${...:name}` fallback with `canonicalId || path`).
- Around line 903-913: The code builds map keys from
`${task.module}:${task.name}` which causes stale-row duplicates; change the key
generation in the loops that iterate this.tasks and this.tools (where key is
currently assigned) to use the stable canonicalId when present, e.g. use const
key = task.canonicalId && task.canonicalId.length ? task.canonicalId :
`${task.module}:${task.name}` (or equivalent) and apply the same pattern
wherever you build keys for tasks, tools, and any task-tool join entries so
lookups/updates use canonicalId first and fall back to module:name only if
canonicalId is absent.
In `@tools/cli/installers/lib/ide/_config-driven.js`:
- Around line 740-748: cleanupTarget() is currently deleting every entry that
starts with "bmad", which removes user-created launchers like those created by
installCustomAgentLauncher(); update the entries loop to exclude the custom
agent pattern (e.g., skip entries that startWith "bmad-custom-agent-") so only
intended "bmad" items are removed; specifically adjust the condition inside the
for-loop that checks entry.startsWith('bmad') to also check
!entry.startsWith('bmad-custom-agent-') (or a equivalent regex) before calling
fs.remove on entryPath.
- Around line 782-800: The ancestor scan currently climbs to the filesystem root
(root) causing false conflicts; change the stopping boundary from root to the
platform workspace root by resolving the Git worktree/top-level directory for
resolvedProject and using that value (e.g., workspaceRoot) in place of root when
walking current; implement a helper that runs git rev-parse --show-toplevel (or
falls back to path.parse(resolvedProject).root) and use its result as the root
boundary for the while loop and candidatePath checks (references:
resolvedProject, current, root, candidatePath, targetDir).
In `@tools/cli/installers/lib/ide/shared/bmad-artifacts.js`:
- Around line 38-53: The standalone-agent branch is including any non-customize
`.md` (e.g., README/SKILL docs) and then assigning manifest-derived canonical
IDs which can create duplicate agents; update this branch (the block using
loadSkillManifest, getCanonicalId and agents.push) to apply the same filters
used by getAgentsFromDir: skip README-style files (e.g., README.md / SKILL.md),
require the file content to contain an `<agent>` token, and preserve the
localskip check — additionally guard against duplicate canonical IDs by tracking
seen IDs before pushing to agents so you don't export a second agent with the
same manifest ID.
---
Nitpick comments:
In
`@src/bmm/workflows/2-plan-workflows/create-ux-design/bmad-skill-manifest.yaml`:
- Around line 1-3: The manifest for the workflow (canonicalId:
bmad-create-ux-design, type: workflow, description: "Plan UX patterns and design
specifications") is missing a schema/version marker; add a schemaVersion field
(e.g., schemaVersion: 1) at the top of the YAML to allow future schema evolution
and automated migration tools to distinguish v1 manifests from later versions;
ensure any manifest-parsing code that reads canonicalId or type can fall back to
schemaVersion when present.
In `@src/bmm/workflows/4-implementation/code-review/bmad-skill-manifest.yaml`:
- Around line 1-3: Add a top-level manifestVersion (or schemaVersion) field to
this manifest (e.g., manifestVersion: "1.0") so consumers can detect format
changes; update the manifest that contains canonicalId: bmad-code-review and
type: workflow to include that field, document that it should follow semantic
versioning and be bumped on breaking changes, and ensure any loader/validator
that parses manifests checks manifestVersion before interpreting fields.
In `@src/bmm/workflows/4-implementation/create-story/bmad-skill-manifest.yaml`:
- Around line 1-3: Update the description field for the workflow with
canonicalId bmad-create-story to use imperative mood for consistency; change the
string value of the description key from "Creates a dedicated story file with
all the context needed for implementation" to "Create a dedicated story file
with all the context needed for implementation" so it matches other manifest
descriptions.
In `@src/bmm/workflows/bmad-quick-flow/quick-dev/bmad-skill-manifest.yaml`:
- Around line 1-3: Update the manifest's description to avoid implying spec
creation: change the description field (canonicalId: bmad-quick-dev) from
"Implement a Quick Tech Spec for small changes or features" to a clearer phrase
such as "Implement code changes from a Quick Tech Spec for small changes or
features" so it's obvious this workflow implements code based on an existing
quick-spec workflow rather than creating the spec.
In `@src/bmm/workflows/bmad-quick-flow/quick-spec/bmad-skill-manifest.yaml`:
- Line 3: The description field in the bmad-skill-manifest.yaml is redundant and
verbose; replace the current value ("Very quick process to create
implementation-ready quick specs for small changes or features") with a concise,
non-redundant phrase such as "Create implementation-ready quick specs for small
changes" by editing the description property in the manifest (the description:
string entry).
In `@src/bmm/workflows/document-project/bmad-skill-manifest.yaml`:
- Line 3: Update the manifest's description field to remove the jargon
"brownfield projects" and use a clearer, broader phrase; edit the description
value in bmad-skill-manifest.yaml (the description: "Document brownfield
projects for AI context" entry) to something like "Document existing codebases
to establish AI context" or similar wording that avoids industry-specific terms
and does not exclude greenfield use cases.
In `@test/test-installation-components.js`:
- Around line 202-223: The temp project created by fs.mkdtemp (tempProjectDir)
is only removed on the success path; wrap the block that uses tempProjectDir
(from the fs.mkdtemp call through all assertions and IdeManager.setup/result
checks) in a try/finally and move the cleanup call (await
fs.remove(tempProjectDir)) into the finally so the temp directory is removed
even on test failures; apply the same try/finally pattern to any other repeated
mkdtemp blocks in this file to ensure no temp dirs are leaked.
- Around line 218-219: The test only asserts the existence of a global SKILL.md
(skillFile) and misses verifying module-scoped agents and globally-installed
tasks/tools; update the tests that create skillFile (using tempProjectDir and
path.join('.windsurf','skills',...,'SKILL.md')) to also assert at least one
agent-specific skill file (e.g., under the agent module name expected when
selectedModules filters apply) and one task/tool skill file that should be
present globally; apply the same pattern to the other occurrences referenced
(the checks around lines 265-266, 312-313, 364-365, 424-425) so each platform
test includes one agent-scoped assertion and one task/tool/global assertion, and
ensure the agent assertions reflect selectedModules scoping while task/tool
assertions validate global installation.
In `@tools/cli/installers/lib/ide/manager.js`:
- Around line 61-65: The hardcoded allowlist in loadCustomInstallerFiles
(variable customFiles) prevents new installer files in __dirname from being
discovered; change to dynamic directory discovery: read the directory contents
(fs.readdir or readdirSync) under __dirname, filter for installer JS files
(e.g., /\.js$/) and exclude a small explicit denylist (e.g., README, index, or
known non-installer filenames) instead of maintaining customFiles; update
loadCustomInstallerFiles to build the final list from the discovered files so
new installers are auto-picked up.
In `@tools/cli/installers/lib/ide/shared/bmad-artifacts.js`:
- Line 38: loadSkillManifest is being called repeatedly causing redundant disk
I/O; add a simple per-directory in-memory cache (e.g., a Map named
skillManifestCache keyed by agentDirPath) and update all call sites that
currently call loadSkillManifest (including the usages that mirror the lines
noted) to first check skillManifestCache.get(agentDirPath) and return the cached
manifest if present, otherwise call loadSkillManifest, store the result in
skillManifestCache.set(agentDirPath, manifest) and then return it; ensure the
cache is module-scoped so loadSkillManifest callers across this file reuse it
and consider optionally invalidating entries if manifest mtime checking is
desired.
In `@tools/cli/installers/lib/ide/shared/workflow-command-generator.js`:
- Around line 90-101: Add JSDoc/type info for the artifact object's canonicalId
so consumers know it's an optional string and any constraints; update the
typedef or JSDoc for the artifact shape (e.g., the WorkflowArtifact typedef or
the JSDoc above collectWorkflowArtifacts) to include a line like "canonicalId?:
string — optional canonical identifier for the workflow, if present must be a
non-empty string" and ensure the documentation appears next to where
artifacts.push(...) is defined (or the function collectWorkflowArtifacts) so
tooling and IDEs pick up the field description and type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d0ee7156-9468-47ab-844c-16702cdfa4d5
📒 Files selected for processing (37)
src/bmm/agents/bmad-skill-manifest.yamlsrc/bmm/agents/tech-writer/bmad-skill-manifest.yamlsrc/bmm/workflows/1-analysis/create-product-brief/bmad-skill-manifest.yamlsrc/bmm/workflows/2-plan-workflows/create-ux-design/bmad-skill-manifest.yamlsrc/bmm/workflows/3-solutioning/check-implementation-readiness/bmad-skill-manifest.yamlsrc/bmm/workflows/3-solutioning/create-architecture/bmad-skill-manifest.yamlsrc/bmm/workflows/3-solutioning/create-epics-and-stories/bmad-skill-manifest.yamlsrc/bmm/workflows/4-implementation/code-review/bmad-skill-manifest.yamlsrc/bmm/workflows/4-implementation/correct-course/bmad-skill-manifest.yamlsrc/bmm/workflows/4-implementation/create-story/bmad-skill-manifest.yamlsrc/bmm/workflows/4-implementation/dev-story/bmad-skill-manifest.yamlsrc/bmm/workflows/4-implementation/retrospective/bmad-skill-manifest.yamlsrc/bmm/workflows/4-implementation/sprint-planning/bmad-skill-manifest.yamlsrc/bmm/workflows/4-implementation/sprint-status/bmad-skill-manifest.yamlsrc/bmm/workflows/bmad-quick-flow/quick-dev-new-preview/bmad-skill-manifest.yamlsrc/bmm/workflows/bmad-quick-flow/quick-dev/bmad-skill-manifest.yamlsrc/bmm/workflows/bmad-quick-flow/quick-spec/bmad-skill-manifest.yamlsrc/bmm/workflows/document-project/bmad-skill-manifest.yamlsrc/bmm/workflows/generate-project-context/bmad-skill-manifest.yamlsrc/bmm/workflows/qa-generate-e2e-tests/bmad-skill-manifest.yamlsrc/core/agents/bmad-skill-manifest.yamlsrc/core/tasks/bmad-skill-manifest.yamlsrc/core/workflows/brainstorming/bmad-skill-manifest.yamlsrc/core/workflows/party-mode/bmad-skill-manifest.yamltest/test-installation-components.jstools/cli/installers/lib/core/manifest-generator.jstools/cli/installers/lib/ide/_config-driven.jstools/cli/installers/lib/ide/codex.jstools/cli/installers/lib/ide/manager.jstools/cli/installers/lib/ide/platform-codes.yamltools/cli/installers/lib/ide/shared/agent-command-generator.jstools/cli/installers/lib/ide/shared/bmad-artifacts.jstools/cli/installers/lib/ide/shared/path-utils.jstools/cli/installers/lib/ide/shared/skill-manifest.jstools/cli/installers/lib/ide/shared/task-tool-command-generator.jstools/cli/installers/lib/ide/shared/workflow-command-generator.jstools/docs/native-skills-migration-checklist.md
💤 Files with no reviewable changes (1)
- tools/cli/installers/lib/ide/codex.js
src/bmm/workflows/3-solutioning/create-architecture/bmad-skill-manifest.yaml
Show resolved
Hide resolved
src/bmm/workflows/3-solutioning/create-epics-and-stories/bmad-skill-manifest.yaml
Show resolved
Hide resolved
src/bmm/workflows/4-implementation/sprint-planning/bmad-skill-manifest.yaml
Show resolved
Hide resolved
…xture The installation component tests walked up the filesystem looking for a pre-installed _bmad directory, which exists locally but not in CI. Replace findInstalledBmadDir() with createTestBmadFixture() that creates a minimal temp directory with fake compiled agents, making tests fully self-contained.
Collaborator
|
@alexeyv doesnt this also need manifests for the workflow.md's also (the ones that do not rely on workflow.xml) |
alexeyv
added a commit
to alexeyv/BMAD-METHOD
that referenced
this pull request
Mar 7, 2026
These manifests were missed during the all-is-skills migration (bmad-code-org#1834), leaving 6 workflows undiscoverable by the native skills installer. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This was referenced Mar 7, 2026
alexeyv
added a commit
that referenced
this pull request
Mar 7, 2026
These manifests were missed during the all-is-skills migration (#1834), leaving 6 workflows undiscoverable by the native skills installer. Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
3 tasks
4 tasks
3 tasks
4 tasks
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
2 participants
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
Convert BMAD method to skills-based architecture with skill manifests.
Status
Draft PR for review and discussion.