feat: add MiniMax Code skills support#1214
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 (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds MiniMax Code as a skills-only AI tool integration with global user-home skill storage. A new ChangesMiniMax Code Global-Skills Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/init.ts (1)
667-676:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSuccess output reports command location incorrectly.
The summary prints command counts using
tool.skillsPath, which can state commands are in the skills directory (not true for adapter-based tools) and is especially misleading forminimax-codewhere commands are skipped.Proposed adjustment
- if (skillCount > 0 && commandCount > 0) { - console.log(`${skillCount} skills and ${commandCount} commands in ${toolDirs}/`); - } else if (skillCount > 0) { + if (skillCount > 0) { console.log(`${skillCount} skills in ${toolDirs}/`); - } else if (commandCount > 0) { - console.log(`${commandCount} commands in ${toolDirs}/`); + } + + const generatedCommandTools = successfulTools.filter( + (t) => !results.commandsSkipped.includes(t.value) && CommandAdapterRegistry.get(t.value) + ); + if (commandCount > 0 && generatedCommandTools.length > 0) { + console.log( + `${commandCount} commands generated for ${generatedCommandTools + .map((t) => t.name) + .join(', ')}` + ); }🤖 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 `@src/core/init.ts` around lines 667 - 676, The output message uses toolDirs (constructed from successfulTools.map((t) => t.skillsPath)) to report the location of both skills and commands, but commands are not necessarily stored in the skills directory. To fix this, build a separate set of directories specifically for commands from the appropriate command path property (not skillsPath), and use toolDirs only when reporting skills, and commandDirs only when reporting commands. This ensures the output accurately reflects where each artifact type is actually located.
🤖 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.
Inline comments:
In `@test/core/shared/skill-paths.test.ts`:
- Line 23: The test assertions in the resolveToolSkillsDir test are using
hard-coded POSIX path separators (forward slashes like /repo/app/.claude/skills)
which will fail on Windows systems. Replace all hard-coded path strings in the
expect assertions with path.join() to construct the expected paths
programmatically, ensuring the tests work across all operating systems.
Specifically, replace `/repo/app/.claude/skills` with path.join('/repo/app',
'.claude', 'skills') and apply the same pattern to other similar hard-coded
paths in related assertions.
In `@test/core/workspace/skills.test.ts`:
- Around line 59-81: The test hard-codes a POSIX path string in the expect
assertion, making it platform-sensitive and missing Windows path separator
coverage. In the test for getWorkspaceSkillDirectory function with
'minimax-code', replace the hard-coded string '/home/alex/.minimax/skills' in
the expect call with a dynamic path constructed using path.join() from the
component parts (the HOME directory and the relative path segments), so the
assertion automatically uses the correct path separators for the platform being
tested.
---
Outside diff comments:
In `@src/core/init.ts`:
- Around line 667-676: The output message uses toolDirs (constructed from
successfulTools.map((t) => t.skillsPath)) to report the location of both skills
and commands, but commands are not necessarily stored in the skills directory.
To fix this, build a separate set of directories specifically for commands from
the appropriate command path property (not skillsPath), and use toolDirs only
when reporting skills, and commandDirs only when reporting commands. This
ensures the output accurately reflects where each artifact type is actually
located.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6fb29e29-71d7-4cd0-bb4d-7c51a2d3994d
📒 Files selected for processing (34)
docs/cli.mddocs/commands.mddocs/supported-tools.mdopenspec/changes/add-minimax-code-tool-support/.openspec.yamlopenspec/changes/add-minimax-code-tool-support/design.mdopenspec/changes/add-minimax-code-tool-support/proposal.mdopenspec/changes/add-minimax-code-tool-support/specs/ai-tool-paths/spec.mdopenspec/changes/add-minimax-code-tool-support/specs/cli-init/spec.mdopenspec/changes/add-minimax-code-tool-support/specs/cli-update/spec.mdopenspec/changes/add-minimax-code-tool-support/specs/workspace-links/spec.mdopenspec/changes/add-minimax-code-tool-support/tasks.mdsrc/cli/index.tssrc/core/available-tools.tssrc/core/config.tssrc/core/init.tssrc/core/migration.tssrc/core/profile-sync-drift.tssrc/core/shared/index.tssrc/core/shared/skill-paths.tssrc/core/shared/tool-detection.tssrc/core/update.tssrc/core/workspace/skills.tstest/commands/config-profile.test.tstest/commands/context-store.test.tstest/commands/workspace.test.tstest/core/available-tools.test.tstest/core/context-store/registry.test.tstest/core/init.test.tstest/core/profile-sync-drift.test.tstest/core/shared/skill-paths.test.tstest/core/shared/tool-detection.test.tstest/core/update.test.tstest/core/workspace/skills.test.tstest/helpers/temp-dir.ts
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
Thanks for the thorough MiniMax integration. The global skill resolver shape and test coverage are directionally good, but I found one user-facing correctness issue that should be fixed before merge.
src/core/init.ts now builds toolDirs from successfulTools.map((t) => t.skillsPath) and uses that same value for both skills and commands in the setup summary. That is wrong for adapter-backed tools because commands are generated under command-specific directories, not the skills directory, and it is especially misleading for minimax-code because command generation is skipped entirely. With delivery: both, openspec init --tools minimax-code can report something like N skills and M commands in ~/.minimax/skills/ and only later say commands were skipped. Please split the success summary so skill counts use skill paths and command counts only include tools with a command adapter and their actual command output locations, or omit the command location/count when every selected tool skipped commands.
The other broad pieces I checked look sane: resolveToolSkillsDir() consistently targets home-backed ~/.minimax/skills, update/drift/detection paths now use shared skill-capable helpers, and the workspace skill tests cover preserving global MiniMax skills during commands-only delivery. The path-separator nits in tests are worth cleaning up too, but this summary output bug is the blocking bit because it tells users OpenSpec generated artifacts that do not exist.
showms
force-pushed
the
codex/add-minimax-code-tool-support
branch
from
eb444bd to
4746cee
Compare
|
@alfred-openspec Thanks for the review. I pushed a follow-up update that splits the init summary for skills and commands.
|
2 participants
Summary
Adds MiniMax Code as a supported OpenSpec tool target.
This integration treats MiniMax Code as a global skills-based tool. OpenSpec now installs and refreshes MiniMax Code skills under the user-home MiniMax skills directory:
~/.minimax/skillsThe tool metadata uses
globalSkillsDir: '.minimax', and the shared skill path resolver appends the standard/skillssuffix, matching the shape of existingskillsDirtool metadata.Changes
minimax-codeto supported AI tools~/.minimax/skillsopenspec init,openspec update, and workspace skill setup/updatecommands.minimaxor.mavisfallback directoriesVerification
pnpm run buildpnpm testFull test suite passes:
Summary by CodeRabbit
New Features
minimax-code) as a supported OpenSpec tool foropenspec init --tools.~/.minimax/skills), skipping command generation..minimax/.mavisfallback creation or deletion).Documentation
Tests