feat(codex): add installer for skills#1714
feat(codex): add installer for skills#1714wsmoak wants to merge 10 commits intobmad-code-org:mainfrom
Conversation
New codex-skills.js writes BMAD artifacts as Agent Skills (directory per skill with SKILL.md) to .agents/skills/. Existing codex.js prompts installer marked as deprecated with preferred=false. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@coderabbitai review |
🤖 Augment PR SummarySummary: Adds a new Codex installer path that exports BMAD artifacts using the agentskills.io “Skill” format, while keeping the legacy Codex custom-prompts flow available but deprecated. Changes:
Technical Notes: Skills are written as directories (skill id == directory name) and each skill’s 🤖 Was this summary useful? React with 👍 or 👎 |
📝 WalkthroughWalkthroughThe PR introduces a new CodexSkillsSetup class that exports BMAD artifacts (agents, tasks, workflows) as Agent Skills in agentskills.io format via CLI. It marks the original Codex setup as deprecated, updates the platform registry, and enables the new skills-based installer to be loaded. Changes
Sequence DiagramsequenceDiagram
participant CLI as CLI Caller
participant Setup as CodexSkillsSetup
participant FS as File System
participant Transform as YAML Transformer
CLI->>Setup: setup(projectDir, bmadDir, options)
Setup->>Setup: collectClaudeArtifacts(projectDir, bmadDir)
Setup->>FS: read artifact files (agents, tasks, workflows)
FS-->>Setup: artifact content
Setup->>Setup: getCodexSkillsDir(projectDir)
Setup->>Setup: clearOldBmadSkills(destDir)
FS->>FS: remove old skill directories
Setup->>Setup: writeSkillArtifacts(destDir, agents)
Setup->>Transform: transformToSkillFormat(content, skillName)
Transform-->>Setup: transformed skill content
Setup->>FS: write SKILL.md files
Setup-->>CLI: return summary (counts, destDir)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
tools/cli/installers/lib/ide/manager.js (1)
11-11: Stale comments still list the old custom installer set.Both this comment and the JSDoc on line 47 omit
codex-skills.js:-// 1. Custom installer files (codex.js, github-copilot.js, kilo.js) - for platforms with unique installation logic +// 1. Custom installer files (codex.js, codex-skills.js, github-copilot.js, kilo.js) - for platforms with unique installation logicAnd line 47:
- * 1. Load custom installer files first (codex.js, github-copilot.js, kilo.js) + * 1. Load custom installer files first (codex.js, codex-skills.js, github-copilot.js, kilo.js)🤖 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` at line 11, Update the stale header comment and the JSDoc that enumerate custom installer filenames to include the missing codex-skills.js entry (the comment currently lists "codex.js, github-copilot.js, kilo.js"); find the file-level comment block and the JSDoc that documents the custom installer set and add "codex-skills.js" to both lists so the documentation matches the actual installer files.tools/cli/installers/lib/ide/codex-skills.js (2)
107-133: An error on one artifact aborts all remaining writes, leaving the skills directory in a partial state.
clearOldBmadSkillsalready wiped the old skills beforewriteSkillArtifactsruns. IfensureDirorwriteFilethrows for any single artifact, subsequent artifacts are never written and the directory is left incomplete with no diagnostics. Add per-artifact error handling consistent withclearOldBmadSkills.🔧 Proposed fix
for (const artifact of artifacts) { if (artifact.type && artifact.type !== artifactType) { continue; } const flatName = toDashPath(artifact.relativePath); const skillName = flatName.replace(/\.md$/, ''); const skillDir = path.join(destDir, skillName); - await fs.ensureDir(skillDir); - - const skillContent = this.transformToSkillFormat(artifact.content, skillName); - - await fs.writeFile(path.join(skillDir, 'SKILL.md'), skillContent, 'utf8'); - writtenCount++; + try { + await fs.ensureDir(skillDir); + const skillContent = this.transformToSkillFormat(artifact.content, skillName); + await fs.writeFile(path.join(skillDir, 'SKILL.md'), skillContent, 'utf8'); + writtenCount++; + } catch (error) { + await prompts.log.warn(` Skipping skill ${skillName}: ${error.message}`); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/codex-skills.js` around lines 107 - 133, The writeSkillArtifacts loop must not abort on a single artifact error; wrap the per-artifact work (ensureDir, transformToSkillFormat, writeFile) in a try/catch so any exception for that artifact is caught, logged the same way clearOldBmadSkills does (use the same logger/syntax used there, e.g., this.logger.error or processLogger.error with contextual info including skillName and the error), increment writtenCount only on success, and continue to the next artifact; preserve the returned writtenCount and do not rethrow unless you want to fail the whole operation explicitly.
308-319:projectDirparameter is accepted but never used.The method only references
destDir. The deadprojectDirparameter suggests unfinished logic and confuses callers.🔧 Proposed fix
- getProjectSpecificInstructions(projectDir = null, destDir = null) { + getProjectSpecificInstructions(destDir = null) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/cli/installers/lib/ide/codex-skills.js` around lines 308 - 319, The parameter projectDir of getProjectSpecificInstructions is unused; update the function to use projectDir when destDir is not provided (e.g., default destDir to `${projectDir}/.agents/skills`) or remove the unused parameter; specifically modify getProjectSpecificInstructions(projectDir = null, destDir = null) so the returned "Skills installed to:" line reflects projectDir when destDir is null (and still falls back to '<project>/.agents/skills' if both are null), keeping the function signature and callers consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/cli/installers/lib/ide/codex-skills.js`:
- Line 128: The fs.writeFile call that writes SKILL.md currently omits the
'utf8' encoding and should be made consistent with installCustomAgentLauncher;
update the await fs.writeFile(path.join(skillDir, 'SKILL.md'), skillContent)
call to pass the 'utf8' encoding option so SKILL.md is written with explicit
UTF-8 encoding (locate the write using skillDir, 'SKILL.md', and skillContent
and mirror the encoding usage from installCustomAgentLauncher).
- Line 46: getTasksFromBmad is being passed options.selectedModules which
incorrectly filters tasks; change the call in codex-skills.js to call
getTasksFromBmad(bmadDir) so tasks are always returned unfiltered, and make the
same change in the sibling codex.js and inside collectClaudeArtifacts (the other
getTasksFromBmad invocation) so only agents remain module-scoped while
tasks/workflows stay global; update any related call sites or tests that assumed
the filtered signature.
- Around line 28-97: The setup() function currently runs generators twice
(collectClaudeArtifacts and then recreates/recalls agents/tasks/workflows) which
duplicates I/O and can make the logged counts diverge from what was actually
written; instead, use the artifacts returned by collectClaudeArtifacts and feed
those into writeSkillArtifacts (split by type) and derive final counts from the
write results. Concretely: stop re-running
AgentCommandGenerator.collectAgentArtifacts, getTasksFromBmad/task processing,
and WorkflowCommandGenerator.collectWorkflowArtifacts; take the artifacts array
from collectClaudeArtifacts, partition by type
('agent'/'task'/'workflow-command'), call writeSkillArtifacts for each
partition, sum their returned written counts into written, and use those write
results to populate the success log and returned counts (still keep mode,
destination, and original artifacts in the return). Ensure functions referenced
are setup, collectClaudeArtifacts, writeSkillArtifacts, AgentCommandGenerator,
TaskToolCommandGenerator, and WorkflowCommandGenerator.
- Around line 158-165: The proposed regex truncates multiline descriptions;
instead remove the regex-based newline collapse and configure yaml.stringify to
emit single-line scalars by passing { lineWidth: 0 } when creating
newFrontmatter (i.e., change the call to yaml.stringify({ name: skillName,
description }, { lineWidth: 0 }) and keep
newFrontmatter/newFrontmatter.trimEnd() logic), ensuring description and
skillName are preserved and no manual newline-truncating regex is used.
In `@tools/cli/installers/lib/ide/codex.js`:
- Line 17: Update the display name passed to the installer constructor so it
matches the entry in platform-codes.yaml: replace the current second argument in
the super call (currently 'Codex (Custom Prompts) (Deprecated)') with the
canonical string 'Codex (Deprecated)' so the constructor invocation
super('codex', 'Codex (Deprecated)', false) is consistent with the YAML catalog.
In `@tools/cli/installers/lib/ide/platform-codes.yaml`:
- Around line 60-65: The YAML entry for platform key codex-skills has a
misleading name/value; update the name to match the custom installer's display
string or add a clarifying comment: change the name field from "Codex" to "Codex
(Skills)" to match the custom installer class/registration (CodexSkillsSetup) so
loadConfigDrivenHandlers no longer treats the YAML fields as dead/ambiguous, or
add a short comment above the codex-skills block stating these YAML fields are
informational only when a custom installer (CodexSkillsSetup) owns the platform.
---
Nitpick comments:
In `@tools/cli/installers/lib/ide/codex-skills.js`:
- Around line 107-133: The writeSkillArtifacts loop must not abort on a single
artifact error; wrap the per-artifact work (ensureDir, transformToSkillFormat,
writeFile) in a try/catch so any exception for that artifact is caught, logged
the same way clearOldBmadSkills does (use the same logger/syntax used there,
e.g., this.logger.error or processLogger.error with contextual info including
skillName and the error), increment writtenCount only on success, and continue
to the next artifact; preserve the returned writtenCount and do not rethrow
unless you want to fail the whole operation explicitly.
- Around line 308-319: The parameter projectDir of
getProjectSpecificInstructions is unused; update the function to use projectDir
when destDir is not provided (e.g., default destDir to
`${projectDir}/.agents/skills`) or remove the unused parameter; specifically
modify getProjectSpecificInstructions(projectDir = null, destDir = null) so the
returned "Skills installed to:" line reflects projectDir when destDir is null
(and still falls back to '<project>/.agents/skills' if both are null), keeping
the function signature and callers consistent.
In `@tools/cli/installers/lib/ide/manager.js`:
- Line 11: Update the stale header comment and the JSDoc that enumerate custom
installer filenames to include the missing codex-skills.js entry (the comment
currently lists "codex.js, github-copilot.js, kilo.js"); find the file-level
comment block and the JSDoc that documents the custom installer set and add
"codex-skills.js" to both lists so the documentation matches the actual
installer files.
alexeyv
left a comment
There was a problem hiding this comment.
A couple of things worth flagging before this merges:
Format mismatch: The installer uses the old flat .md files in .codex/prompts/ — the deprecated custom prompts format. The new Skills format requires a directory-per-skill layout (skill-name/SKILL.md) with name matching the directory name. The PR title says "skills" but it's really a location picker for the old prompts format.
Unclear choice: Both installation options use the same .codex/prompts/ path convention — the only difference is the base directory. The project-specific option also silently requires a CODEX_HOME env var setup that isn't surfaced in the UI. Some context on the tradeoff would help users choose.
Implicit invocation: If/when the files are migrated to the real Skills format, it's worth setting allow_implicit_invocation: false in each skill's front matter, to be consistent with the Claude Code installer.
Last but not least, maybe not in this PR, but we should get some sort of AbstractSkillsInstaller / ClaudeSkillsInstaller / CodexSkillsIstaller / EveryoneAndTheirMotherSkillsInstaller construct going on. They will all be following Anthropic skills standard, but going into a different directory with different front matter and probably some other unexpected quirks.
|
Are you thinking of I don't understand the first two comments. Skills are installed to .codex/skills in directories as SKILL.md files, not as files in .codex/prompts. Closing, superseded by #1729 |




What
Add separate installer for Codex Skills
Why
Related to #1393
Supersedes #1658
How
Testing
Concerns
This intentionally introduces duplication to avoid touching any other installers and is not intended to be the final implementation of Skills for BMad.
It will look for the workflow files in _bmad relative to the skill directory and when it doesn't find them, it falls back to looking in the project. This works, but is not optimal. Eventually, skills probably need to be self-contained. At that point they will likely be getting distributed in some other way.
As a workaround, add a line to AGENTS.md saying the following will prevent the extra search: