feat: add Oh My Pi (omp) tool support - #1313
Conversation
|
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 (8)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR adds support for the Oh My Pi (omp) tool by registering it in configuration and the command adapter registry, implementing command-file formatting for ChangesOh My Pi tool integration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/core/command-generation/adapters.test.ts (1)
594-639: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding YAML-escaping coverage for
ompAdapter.
piAdapterandbobAdaptertest suites coverescapeYamlValueedge cases (special chars, newlines, empty description), but the newompAdaptersuite doesn't, despite using the sameescapeYamlValuehelper for itsdescriptionfield.✅ Suggested additional test cases
it('should inject template arguments into the input section', () => { const contentWithInput: CommandContent = { ...sampleContent, body: '**Input**: The argument after `/opsx:explore` is the topic.\n\n**Steps**\n1. Think.', }; const output = ompAdapter.formatFile(contentWithInput); expect(output).toContain('**Provided arguments**: $@'); }); + + it('should escape YAML special characters in description', () => { + const contentWithSpecialChars: CommandContent = { + ...sampleContent, + description: 'Fix: regression in "auth" feature', + }; + const output = ompAdapter.formatFile(contentWithSpecialChars); + expect(output).toContain('description: "Fix: regression in \\"auth\\" feature"'); + }); + + it('should escape newlines in description', () => { + const contentWithNewline: CommandContent = { + ...sampleContent, + description: 'Line 1\nLine 2', + }; + const output = ompAdapter.formatFile(contentWithNewline); + expect(output).toContain('description: "Line 1\\nLine 2"'); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/core/command-generation/adapters.test.ts` around lines 594 - 639, The ompAdapter test suite is missing coverage for the shared escapeYamlValue behavior used when writing the description frontmatter. Add a few focused assertions in adapters.test.ts around ompAdapter.formatFile and/or escapeYamlValue to cover special characters, multiline descriptions, and empty descriptions, using ompAdapter and sampleContent so the new adapter matches the existing piAdapter and bobAdapter YAML-escaping coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/core/command-generation/adapters.test.ts`:
- Around line 594-639: The ompAdapter test suite is missing coverage for the
shared escapeYamlValue behavior used when writing the description frontmatter.
Add a few focused assertions in adapters.test.ts around ompAdapter.formatFile
and/or escapeYamlValue to cover special characters, multiline descriptions, and
empty descriptions, using ompAdapter and sampleContent so the new adapter
matches the existing piAdapter and bobAdapter YAML-escaping coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 24e5c826-5312-446e-a1dd-8fb0a76df0f3
📒 Files selected for processing (8)
.changeset/add-oh-my-pi-tool.mddocs/supported-tools.mdsrc/core/command-generation/adapters/index.tssrc/core/command-generation/adapters/omp.tssrc/core/command-generation/registry.tssrc/core/config.tstest/core/available-tools.test.tstest/core/command-generation/adapters.test.ts
Oh My Pi (omp) is a distinct agent from the existing `pi` (pi.dev). It discovers Agent Skills from `.omp/skills` (invoked as `/skill:<name>`) and file-based slash commands from `.omp/commands` (invoked as `/opsx-<name>`). Register `omp` with `skillsDir: '.omp'` plus an `omp` command adapter (mirrors the Pi adapter, writing `.omp/commands/opsx-<id>.md`), so it ships a full skills + slash-command integration. Adds docs, a tool-detection test, and adapter coverage. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
e5f4be4 to
c98ed03
Compare
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Reviewed the OMP integration against OMP's actual discovery semantics. The and layouts match native OMP behavior, and the adapter/tests cover the important command reference, args, and YAML cases. Looks good to me.
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Reviewed the OMP integration against OMP actual discovery semantics. The .omp/skills//SKILL.md and .omp/commands/opsx-.md layouts match native OMP behavior, and the adapter/tests cover the important command reference, args, and YAML cases. Looks good to me.
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Reviewed against OpenSpec command-generation patterns and the current Oh My Pi docs surface. The implementation matches the existing adapter strategy: project skills under .omp/skills, project slash commands under .omp/commands/opsx-.md, hyphen command references, and argument injection. I also verified the focused available-tools and command-adapter tests pass locally in a temp checkout. Looks good to merge.
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Reviewed the OMP command/skill paths against current Oh My Pi docs. The layout, description frontmatter, filename-derived invocation, argument handling, and skill layout all line up with OMP's native discovery model. I don't see a blocking adapter issue.
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Re-ran the OMP adapter review against the current diff. I still don't see a blocker: the .omp/skills/openspec-*/SKILL.md and .omp/commands/opsx-<id>.md outputs match OMP's documented discovery shape, command references are converted to /opsx-*, and argument plus YAML escaping coverage is present. Safe to merge from my side.
|
Thanks for putting this together and helping get Oh My Pi support across the line. We just merged Oh My Pi support in #1276, which adds the |
2 participants
Summary
omp, https://github.com/oh-my-pi/pi-coding-agent) as a supported tool. Oh My Pi is a distinct agent from the existingpi(pi.dev): it discovers Agent Skills from.omp/skills(invoked as/skill:<name>) and file-based slash commands from.omp/commands(invoked as/opsx-<name>).ompwithskillsDir: '.omp'and adds anompcommand adapter (mirrors the Pi adapter, writing.omp/commands/opsx-<id>.md), so it ships a full skills + slash-command integration like Claude/Codex/OpenCode. (Note: unlike the recent Mistral Vibe addition, which was skills-only, omp has a native slash-command surface, so a command adapter is included.)docs/supported-tools.mdand adds tool-detection + adapter test coverage.Validation
openspec init --tools ompwrites 5 skills under.omp/skills/and 5 command templates under.omp/commands/; verified in Oh My Pi they load as/skill:openspec-*and/opsx-*(confirmed via real invocation).pnpm run buildpnpm test— 1815 passingpnpm run lint— clean (0 errors)Summary by CodeRabbit
omp) in the available tool list and built-in command generation..omp/commands/opsx-<id>.mdpattern, including proper command reference formatting and optional argument injection.omptool and validating command generation behavior.