feat: add pi coding agent as supported platform#1854
feat: add pi coding agent as supported platform#1854bmadcode merged 2 commits intobmad-code-org:mainfrom
Conversation
Add pi (provider-agnostic terminal-native AI coding agent) to platform-codes.yaml with native skills format output to .pi/skills/. Pi follows the open Agent Skills specification and uses the same subdirectory/SKILL.md structure that BMAD already generates for other platforms. Fixes bmad-code-org#1853
📝 WalkthroughWalkthroughAdded support for the "pi" platform, a provider-agnostic terminal-native AI coding agent. Changes include configuration metadata in the platform registry and test suite to validate Pi native skill installation, ensuring proper directory structure and frontmatter validation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
test/test-installation-components.js (2)
1508-1539: These temp directories still leak on the first failed assertion.Both
remove()calls live on the success path. Once any assertion aftermkdtemp()fails, the catch records the failure but leaves the temp project and fixture behind. Put cleanup in afinally.🤖 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 1508 - 1539, The temp directories created with fs.mkdtemp() (tempProjectDir28) and createTestBmadFixture() (installedBmadDir28) are only removed on the success path; move their cleanup into a finally block so they are removed regardless of assertion failures. Specifically, after creating tempProjectDir28 and installedBmadDir28 in the try, declare them in the outer scope and in a finally await fs.remove(tempProjectDir28) and await fs.remove(installedBmadDir28) (guarding existence and swallowing/remove errors safely) so cleanup always runs; keep the existing assertions and the try/catch around the test logic (clearCache, loadPlatformCodes, IdeManager/ensureInitialized, ideManager.setup, SKILL.md checks) but ensure removal happens in finally.
1519-1534:skill_formatdetection is completely untested.For skill-format platforms, detection follows a different code path than installation. This suite should assert Pi is not detected before install and is detected after
.pi/skills/bmad-*exists, otherwise a regression indetect()will slip through whilesetup()still passes.🤖 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 1519 - 1534, Before asserting install results, call IdeManager.detect on the temp project to assert Pi is NOT detected, then after running IdeManager.setup assert that IdeManager.detect now reports Pi detected; specifically use the IdeManager.detect method (or the project-detection entry on the IdeManager instance used for the test) to check detection state before setup and again after setup when .pi/skills/bmad-* has been created, so the test verifies the separate skill_format detection path in addition to setup() success.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/test-installation-components.js`:
- Around line 1519-1526: The test currently only writes YAML and calls
IdeManager.setup('pi', ...), but does not verify that the platform is actually
selectable; call IdeManager.getAvailableIdes() (on the same IdeManager instance,
e.g., ideManager28.getAvailableIdes()) after ensureInitialized() and assert that
the returned list includes 'pi' before calling setup(), so the test proves 'pi'
is exposed as a supported platform; keep the existing setup/assert for success
afterwards.
- Around line 1531-1534: The test currently uses
skillContent28.match(/^name:\s*(.+)$/m) which can match a name: line anywhere;
instead extract the YAML frontmatter between the opening and closing '---'
markers from skillContent28 (e.g., capture the block between the first and
second '---'), then parse or inspect that block to find the name key and compare
it to 'bmad-master' (or use a YAML parser like js-yaml to load the frontmatter
and then assert that frontmatter.name === 'bmad-master'); update the variables
skillContent28/nameMatch28 assertions to operate on the frontmatter content
rather than the whole file body and keep reference to skillFile28 for locating
the file.
- Around line 1531-1534: Add assertions to verify the description frontmatter
and ensure no extra frontmatter keys were leaked: after reading skillFile28 into
skillContent28 (the same block using nameMatch28), extract a description with a
regex like /^description:\s*(.+)$/m and assert it exists and the trimmed value
is non-empty; additionally scan frontmatter lines (e.g., all matches of
/^([A-Za-z0-9_-]+):/m) and assert the set equals exactly ['name','description']
to confirm transformToSkillFormat() normalized the header to only those fields.
- Around line 1510-1514: The test must assert the platform's template_type is
explicitly the default skill template so future changes don't silently pass;
update the assertions around loadPlatformCodes / platformCodes28 / piInstaller
to check piInstaller?.template_type === DEFAULT_SKILL_TEMPLATE (import or
reference the constant that defines the default skill template in your codebase,
or call the function getDefaultSkillTemplate() if present) and if that constant
is not available, assert the concrete expected default value used elsewhere in
the codebase; keep the existing target_dir and skill_format assertions.
- Around line 1528-1534: The test currently only checks SKILL.md exists and that
the frontmatter name matches; enhance the validation by asserting the main skill
payload (the body after the frontmatter) is preserved and non-empty: using the
existing variables skillFile28, skillContent28 and nameMatch28, split
skillContent28 into frontmatter and body (e.g., by locating the frontmatter
match end or the first blank line) and add assertions that the body length is >
0 and that it contains the expected command/content snippet (or exact expected
text loaded from the source fixture) so the test fails if the file has only a
header or truncated content.
---
Nitpick comments:
In `@test/test-installation-components.js`:
- Around line 1508-1539: The temp directories created with fs.mkdtemp()
(tempProjectDir28) and createTestBmadFixture() (installedBmadDir28) are only
removed on the success path; move their cleanup into a finally block so they are
removed regardless of assertion failures. Specifically, after creating
tempProjectDir28 and installedBmadDir28 in the try, declare them in the outer
scope and in a finally await fs.remove(tempProjectDir28) and await
fs.remove(installedBmadDir28) (guarding existence and swallowing/remove errors
safely) so cleanup always runs; keep the existing assertions and the try/catch
around the test logic (clearCache, loadPlatformCodes,
IdeManager/ensureInitialized, ideManager.setup, SKILL.md checks) but ensure
removal happens in finally.
- Around line 1519-1534: Before asserting install results, call
IdeManager.detect on the temp project to assert Pi is NOT detected, then after
running IdeManager.setup assert that IdeManager.detect now reports Pi detected;
specifically use the IdeManager.detect method (or the project-detection entry on
the IdeManager instance used for the test) to check detection state before setup
and again after setup when .pi/skills/bmad-* has been created, so the test
verifies the separate skill_format detection path in addition to setup()
success.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d61a2305-5882-42dd-a4ea-eb4c0e96bff0
📒 Files selected for processing (2)
test/test-installation-components.jstools/cli/installers/lib/ide/platform-codes.yaml
- Assert template_type === 'default' to pin config contract - Verify Pi appears in getAvailableIdes() list - Test detect() returns false before install, true after - Parse frontmatter between --- delimiters instead of regex on full file - Assert description is present and non-empty - Assert frontmatter contains only name and description keys - Validate body content is non-empty with expected activation instructions - Add reinstall/upgrade coverage (rerun setup over existing output) - Move temp directory cleanup to finally block
alexeyv
left a comment
There was a problem hiding this comment.
Now that it's all skills, adding platforms that support them is actually easy. All I'm wondering is how many can we take before the installer UX it's overwhelmed by too many choices. :)
Before long this will probably become "Claude, Cursor, Codex or type where you want them" affair, with a link to a documentation page that's like "correct answers for 5000 different Agentic platforms" :)
What
Add pi (provider-agnostic terminal-native AI coding agent) as a supported platform in the BMAD installer.
Why
Pi follows the open Agent Skills specification and uses the same
subdirectory/SKILL.mdstructure that BMAD already generates for other platforms. Terminal-first developers using pi currently have no native BMAD support and must manually convert command files.Fixes #1853
How
pientry toplatform-codes.yamlwithtarget_dir: .pi/skills,template_type: default, andskill_format: truetest-installation-components.jswith 16 assertions covering:target_dir,skill_format,template_type)getAvailableIdes()includes pi)detectInstalledIdes()before and after install)---markers, not regex on full file)name+descriptionkeys, both valid)setup()over existing output)finallyblockTesting
npm testpasses clean: schemas, refs, install, lint, markdown lint, prettierbmad-cli install --modules bmm --tools pi --yes, confirmed pi discovers and loads all 44 generated skills with no warnings