Conversation
… skill generation Add two new CLI options to enable non-interactive skill generation for CI/CD pipelines and automation scripts: - `--skill-output <path>`: Specify skill output directory path directly, skipping the interactive location prompt - `-f, --force`: Force overwrite existing skill directory without confirmation This addresses #1012 where users needed to generate skills in automated environments without interactive prompts. Example usage: ```bash repomix --skill-generate --skill-output .codebuddy --force ```
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds non-interactive skill generation by introducing Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (CLI)
participant Action as defaultAction / remoteAction
participant Validate as Validation Logic
participant SkillPrompts as skillPrompts (resolve/prepare/getLocation)
participant FS as FileSystem
rect rgb(240,248,255)
Note over User,FS: Non-interactive flow (--skill-output provided)
User->>Action: Run with --skill-output [--force]
Action->>Validate: validateConflictingOptions(...)
Validate-->>Action: validation result
Action->>SkillPrompts: resolveAndPrepareSkillDir(skillOutput, cwd, force)
SkillPrompts->>FS: fs.access/fs.rm checks (via deps)
alt dir exists
SkillPrompts->>FS: rm (if force=true)
FS-->>SkillPrompts: rm result
else dir exists & force=false
SkillPrompts-->>Action: throw RepomixError
else dir absent
SkillPrompts-->>Action: resolved path (no-op)
end
Action->>SkillPrompts: getSkillLocation(resolvedPath)
SkillPrompts-->>Action: 'personal' or 'project'
Action->>Action: set cliOptions.skillDir / adjust copy behavior
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
Summary of ChangesHello @yamadashy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the command-line interface by adding two new options, Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1022 +/- ##
==========================================
- Coverage 90.42% 90.34% -0.08%
==========================================
Files 123 123
Lines 9430 9457 +27
Branches 1699 1709 +10
==========================================
+ Hits 8527 8544 +17
- Misses 903 913 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Deploying repomix with
|
| Latest commit: |
6ecce38
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b4900674.repomix.pages.dev |
| Branch Preview URL: | https://feat-skill-generate-non-inte.repomix.pages.dev |
PR Review: feat(cli): Add --skill-output and --force options for non-interactive skill generationThis PR adds two CLI options ( ✅ Positives
📋 Detailed FeedbackCode Quality1. Consider validating both options together in The current validation checks if (cliOptions.skillOutput && config.skillGenerate === undefined) {
throw new RepomixError('--skill-output can only be used with --skill-generate');
}
if (cliOptions.force && config.skillGenerate === undefined) {
throw new RepomixError('--force can only be used with --skill-generate');
}This is fine, but you could consolidate into a single check for slightly cleaner code: if ((cliOptions.skillOutput || cliOptions.force) && config.skillGenerate === undefined) {
throw new RepomixError('--skill-output and --force can only be used with --skill-generate');
}However, the current approach gives more specific error messages which is arguably better for UX. 2. Minor: Duplicate path resolution logic The path resolution logic is duplicated in both skillDir = path.isAbsolute(cliOptions.skillOutput)
? cliOptions.skillOutput
: path.resolve(process.cwd(), cliOptions.skillOutput);Consider extracting this to a shared utility function in Test CoverageThe new
Potential Issues1. The 2. Missing validation in Unlike Premortem Analysis🔮 Potential Failure Scenarios & Mitigations
SummaryThe PR is well-implemented and ready for merge with minor suggestions. The new options will be valuable for CI/CD automation scenarios. Great work! 👍 |
There was a problem hiding this comment.
Code Review
This pull request adds useful --skill-output and --force options for non-interactive skill generation, which is a great enhancement for CI/CD and automation workflows. The implementation is generally well-done, including new tests for the directory preparation logic. My review focuses on a couple of potential bugs: one related to incorrect path matching for personal skills, and another concerning error handling that might hide filesystem issues. I've also included some suggestions to refactor the code for better maintainability by leveraging commander.js features and reducing duplication.
Update description to indicate it's a general confirmation skip flag, not specific to skill generation only.
PR Review: Additional FeedbackI've reviewed this PR after the previous reviews. The implementation is solid and addresses the CI/CD automation use case well. Most major points have been covered, but I want to highlight one potential bug and a few additional observations: 🐛 Potential Bug:
|
- Fix skillLocation detection using getSkillBaseDir for accurate path matching - Improve error handling in prepareSkillDir to re-throw non-ENOENT errors - Extract resolveAndPrepareSkillDir helper to reduce code duplication - Add getSkillLocation helper for consistent location type detection - Add tests for new error handling and getSkillLocation function
PR Review: Additional ObservationsI've reviewed this PR after previous reviews from CodeRabbit, Gemini, and other Claude reviews. The implementation is well-structured and addresses the CI/CD automation use case effectively. Here are a few new observations not covered in previous reviews: ✅ What's Working Well
🔍 New Observations1. Missing validation in remoteAction.tsUnlike // In remoteAction.ts, before line 103:
if ((cliOptions.skillOutput || cliOptions.force) && cliOptions.skillGenerate === undefined) {
throw new RepomixError('--skill-output and --force can only be used with --skill-generate');
}This provides consistent behavior and error messages regardless of which action path is taken. 2. Error message propagation for RepomixErrorIn // Line 116 - correctly re-throws RepomixError because error.code !== 'ENOENT'
if ((error as NodeJS.ErrnoException)?.code !== 'ENOENT') {
throw error;
}The logic works correctly because 3. Consider adding resolveAndPrepareSkillDir to test coverageWhile describe('resolveAndPrepareSkillDir', () => {
test('should resolve relative path from cwd', async () => {
// Test that relative paths are resolved correctly
});
test('should use absolute path as-is', async () => {
// Test that absolute paths are not modified
});
});📊 Test Coverage NoteThe Codecov report shows 60.78% patch coverage (below the 80% target). The gaps are in the action files ( Overall, this is a solid implementation that will be valuable for CI/CD automation. The suggestions above are minor improvements. 👍 |
When using --skill-output with a custom path (e.g., `aaa` instead of `.claude/skills/...`), the skill directory was not being created correctly for remote repositories. Two issues were fixed: 1. defaultAction was re-calculating skillDir even when already set by remoteAction, causing the path to be resolved relative to tempDir instead of cwd 2. copySkillOutputToCurrentDirectory was being called unnecessarily for --skill-output paths, looking in .claude/skills/ when the skill was written directly to the specified path
PR Review: Additional Follow-up ObservationsI've reviewed this PR after the previous reviews (Claude, Gemini, CodeRabbit). The implementation is well-structured and addresses the CI/CD automation use case effectively. Most major points have been covered, but I want to highlight one potential bug that hasn't been addressed: 🐛 Bug: Error Handling Logic in
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces --skill-output and --force options to enable non-interactive skill generation, which is a valuable feature for CI/CD and automation scripts. The changes are well-implemented across the CLI, actions, and supporting functions, including the addition of relevant tests. My review includes a suggestion to refactor a new function for better readability and points out a potential pre-existing issue in the remote action's skill-copying logic that this PR touches upon.
The copySkillOutputToCurrentDirectory function was never actually used because skills are written directly to their final destination via the skillDir absolute path, not to the temp directory. - Remove copySkillOutputToCurrentDirectory function - Remove related tests - Simplify the copy logic in runRemoteAction - Remove unused skillLocation variable and imports
PR Review: Additional ObservationsI've reviewed this PR after the previous reviews. The implementation is solid and well-structured for CI/CD automation use cases. Most points have been well-covered by prior reviewers, so I'll focus on incremental observations. ✅ Strengths
📌 Minor Observations1. Unused function: `getSkillLocation` is defined but never usedThe export const getSkillLocation = (skillDir: string): SkillLocation => {
const personalSkillsBase = getSkillBaseDir('', 'personal');
return skillDir.startsWith(personalSkillsBase) ? 'personal' : 'project';
};If this is intentionally added for future use, consider adding a comment to clarify. Otherwise, it could be removed to avoid unused code. 2. Test coverage gap for action filesPer Codecov, patch coverage is 67.92% (below the 80% target):
While
3. CLI option groupingThe new options are in the main options list rather than grouped with .option('--skill-generate [name]', '...')
.option('--skill-output <path>', '... (requires --skill-generate)')
.option('-f, --force', '... (requires --skill-generate)')Adding the 🔍 Premortem Analysis
SummaryThis PR is well-implemented and ready for merge. The test coverage gap in action files is the main area for improvement, though the core logic is well-tested. Nice work! 👍 |
…ling Address review feedback: - Add empty string validation for --skill-output option - Add file vs directory check in prepareSkillDir to prevent silent file deletion - Add tests for resolveAndPrepareSkillDir function
PR Review: Additional Observations (Post-Latest Commits)The latest commits (Dec 30) have addressed key feedback from previous reviews:
The implementation is now cleaner and more robust. A few remaining observations: 📋 New Observations1. Unused
|
|
when will this be released? :D |
Add release notes for v1.11.1 covering: - Multi-root directory labels (#1024) - Non-interactive skill generation options (#1022) - Remote git command timeout fix (#1078) - CLI output visibility fix for light themes (#1088) - Library bundling documentation (#1075) Improve release note generation guidelines: - Clarify "What's New" vs "Improvements" usage - Add rule to include related issue numbers with PRs - Add rule to include links in documentation sections
Add two new CLI options to enable non-interactive skill generation for CI/CD pipelines and automation scripts (addresses #1012):
--skill-output <path>: Specify skill output directory path directly, skipping the interactive location prompt-f, --force: Skip all confirmation prompts (currently: skill directory overwrite)Usage Examples
Changes
skillOutputandforceoptions to CLI types--skill-outputand-f, --forceCLI options in "Skill Generation" groupprepareSkillDir()function to handle directory preparation with force overwritedefaultAction.tsandremoteAction.tsto support non-interactive modeprepareSkillDir()Checklist
npm run testnpm run lint