feat(skill): Improve skill generation for remote repositories#1001
feat(skill): Improve skill generation for remote repositories#1001
Conversation
- Fix project name generation to use repository name from URL instead of temp directory name (e.g., "Vite" instead of "Repomix HPkbgH") - Add source URL to SKILL.md footer for remote repositories only (local repos don't include URL to avoid exposing personal paths) - Copy only .claude/skills/ directory to prevent conflicts with repository's own .claude config (commands, agents, etc.) Example SKILL.md footer: - Remote: "This skill was generated by Repomix from [https://github.com/...]" - Local: "This skill was generated by Repomix"
Deploying repomix with
|
| Latest commit: |
670e839
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://6dc6c2ae.repomix.pages.dev |
| Branch Preview URL: | https://fix-skill-generate-project-n.repomix.pages.dev |
|
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. WalkthroughThis PR extends the skill generation pipeline to compute project names from repository URLs and thread source URL metadata through the skill packaging stack. It restricts Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 significantly enhances the Highlights
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
|
PR Review: Improve skill generation for remote repositoriesThis is a well-structured PR that addresses important usability improvements for skill generation from remote repositories. The implementation is clean and the test coverage is comprehensive. ✅ Strengths
🎯 Code Quality ObservationsMinor suggestions for consideration
🔍 Premortem AnalysisPotential edge cases to considerRisk 1: URL parsing edge cases
Risk 2: Concurrent skill generation
Risk 3: Large repository handling
✅ Overall AssessmentThis PR is ready to merge. The implementation is solid, addresses real user pain points, and maintains backward compatibility. The code is clean, well-tested, and follows the project conventions. The selective copying of only .claude/skills/ is a particularly thoughtful solution that prevents unintended side effects. Great work on this improvement! 🚀 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to capture and utilize a skillProjectName and skillSourceUrl when generating skills, particularly for remote repositories. The changes involve propagating these new parameters through the CLI actions, worker, and packager components, and updating the skill generation logic to use them. A new utility function, generateProjectNameFromUrl, was added to derive a human-readable project name from a repository URL. The SKILL.md template was updated to display this project name in the skill description and include an optional source URL link in the footer. Additionally, the skill output copying mechanism was refined to only copy the .claude/skills directory to prevent conflicts. Review comments suggest improving the SKILL.md source URL link text by using the projectName instead of the full URL, and refactoring the Title Case conversion logic into a shared helper function to reduce code duplication.
Add unit tests for the copySkillOutputToCurrentDirectory function: - Copy .claude/skills directory when it exists - Skip copy when directory does not exist - Handle EPERM and EACCES permission errors - Handle generic errors
![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 (2)
tests/core/skill/skillUtils.test.ts (1)
3-8: Fix the “kebab-case” test case + add a couple of high-value URL edge cases.
- Line 99-101: the test claims kebab-case but uses
repomix(not kebab-case). Use something likemy-project-nameto actually exercise the conversion.- Consider adding coverage for trailing slash + ssh form (
git@github.com:org/repo.git) sinceextractRepoNamedocs mention it.Example patch:
- test('should handle kebab-case repo names', () => { - expect(generateProjectNameFromUrl('https://github.com/yamadashy/repomix')).toBe('Repomix'); - }); + test('should handle kebab-case repo names', () => { + expect(generateProjectNameFromUrl('https://github.com/user/my-project-name')).toBe('My Project Name'); + }); + + test('should handle trailing slash', () => { + expect(generateProjectNameFromUrl('https://github.com/vitejs/vite/')).toBe('Vite'); + }); + + test('should handle ssh URL', () => { + expect(generateProjectNameFromUrl('git@github.com:yamadashy/repomix.git')).toBe('Repomix'); + });Also applies to: 94-115
src/core/skill/skillStyle.ts (1)
3-12: Avoid{{{sourceUrl}}}in markdown link to prevent markdown injection; validate + render safely.
Right nowsourceUrlis inserted unescaped into both the link label and URL. A malicious value could break the markdown structure of SKILL.md. Prefer:
- Validate
sourceUrlupstream (e.g.,new URL(...), allow onlyhttp:/https:), and- Render with a safer markdown form like an autolink:
from <{{sourceUrl}}>(still after validation).Template-only tweak (assuming upstream validation):
-This skill was generated by [Repomix](https://github.com/yamadashy/repomix){{#if sourceUrl}} from [{{{sourceUrl}}}]({{{sourceUrl}}}){{/if}} +This skill was generated by [Repomix](https://github.com/yamadashy/repomix){{#if sourceUrl}} from <{{sourceUrl}}>{{/if}}Also applies to: 99-103
🧹 Nitpick comments (3)
website/client/src/public/schemas/1.10.0/schema.json (1)
2-3: Tighten numeric fields (integer/min) + consider$id/https for schema URL.
Several “count/length/size” fields are currently"number"with no constraints, so configs like-1,1.5, etc. would validate. Suggest making these"integer"with"minimum": 0(e.g.,input.maxFileSize,output.topFilesLength,output.git.sortByChangesMaxCommits,output.git.includeLogsCount). Also consider adding a root-level"$id"and using anhttps$schemaURL for best practice.Example (illustrative) patch:
- "$schema": "http://json-schema.org/draft-07/schema#", + "$schema": "https://json-schema.org/draft-07/schema#", + "$id": "https://repomix.dev/schemas/1.10.0/schema.json", ... - "maxFileSize": { "type": "number" } + "maxFileSize": { "type": "integer", "minimum": 0 } ... - "topFilesLength": { "type": "number" } + "topFilesLength": { "type": "integer", "minimum": 0 } ... - "sortByChangesMaxCommits": { "type": "number" } + "sortByChangesMaxCommits": { "type": "integer", "minimum": 0 } - "includeLogsCount": { "type": "number" } + "includeLogsCount": { "type": "integer", "minimum": 0 }Also applies to: 11-13, 59-61, 96-99, 105-107, 153-155
src/cli/actions/workers/defaultActionWorker.ts (1)
58-60: Good propagation ofskillProjectName/skillSourceUrlintopackOptions.
Optional nit: consider omitting undefined keys to keep logs/debug output cleaner (esp. if options are ever serialized).tests/core/skill/packSkill.test.ts (1)
236-279: Source URL inclusion/omission tests match the conditional rendering intent.
Optional: assert the full rendered link form (not just the label) once the SKILL.md footer format is finalized (especially if you adopt autolink<...>for safety).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/cli/actions/remoteAction.ts(3 hunks)src/cli/actions/workers/defaultActionWorker.ts(1 hunks)src/cli/types.ts(1 hunks)src/core/packager.ts(1 hunks)src/core/skill/packSkill.ts(6 hunks)src/core/skill/skillStyle.ts(3 hunks)src/core/skill/skillUtils.ts(1 hunks)tests/core/skill/packSkill.test.ts(5 hunks)tests/core/skill/skillUtils.test.ts(2 hunks)website/client/src/public/schemas/1.10.0/schema.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
tests/core/skill/packSkill.test.ts (1)
src/core/skill/packSkill.ts (3)
generateSkillReferences(70-143)SkillReferencesResult(38-48)generateSkillMdFromReferences(149-168)
tests/core/skill/skillUtils.test.ts (1)
src/core/skill/skillUtils.ts (1)
generateProjectNameFromUrl(75-83)
src/core/skill/packSkill.ts (1)
src/core/skill/skillUtils.ts (1)
generateProjectName(50-59)
src/cli/actions/remoteAction.ts (1)
src/core/skill/skillUtils.ts (2)
generateDefaultSkillNameFromUrl(115-119)generateProjectNameFromUrl(75-83)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (13)
src/core/packager.ts (1)
50-55: PackOptions extension looks consistent with the new skill metadata flow.
No concerns with addingskillProjectName?: stringandskillSourceUrl?: stringhere.tests/core/skill/packSkill.test.ts (1)
36-47: Coverage forskillProjectNameoverride looks solid.
The temp-dir example is a good regression guard for the remote flow.Also applies to: 73-84, 89-119
src/cli/types.ts (1)
58-64: Types look fine; verify these fields aren’t user-settable in a way that leaks local paths.
The comments say “internal / remote only”; worth double-checking there’s no CLI flag or config option that could populateskillSourceUrlon local runs (or accept afile://...URL).#!/bin/bash set -euo pipefail # Check if these options are exposed as CLI flags or config-mapped fields. rg -n --hidden -S "skillProjectName|skillSourceUrl" -C2src/cli/actions/remoteAction.ts (4)
10-10: LGTM!The import of
generateProjectNameFromUrlis correct and necessary for the new project name generation feature.
97-105: LGTM!The project name generation logic is well-placed within the skill generation block and properly extracts a user-friendly project name from the repository URL. The comment clearly explains the purpose.
113-116: LGTM!The conditional assignment of
skillSourceUrlcorrectly ensures that the repository URL is only propagated when skill generation is active. The spreading of options properly passes the new fields through the pipeline.
216-250: Excellent improvement to prevent conflicts.The refinement to copy only
.claude/skills/instead of the entire.claude/directory is well-reasoned and properly implemented. All path references, checks, and error messages have been consistently updated to reflect the narrower scope. The comments clearly explain the rationale for preventing conflicts with a repository's own.claudeconfiguration.src/core/skill/packSkill.ts (6)
47-47: LGTM!The
sourceUrlfield is properly typed as optional and follows the interface's conventions.
78-79: LGTM!The optional parameters are properly typed and positioned, maintaining backward compatibility while enabling the new metadata propagation feature.
88-89: LGTM!The fallback logic is correct and well-commented. Using the nullish coalescing operator properly handles the case where
skillProjectNameis provided (from URL for remote repos) versus generated (from directory for local repos).
141-141: LGTM!The
sourceUrlis properly propagated through the skill references result.
161-161: LGTM!The
sourceUrlis correctly passed from the references result to the skill MD generation function, completing the propagation pipeline.
220-234: LGTM!The function call properly passes the new
skillProjectNameandskillSourceUrlparameters from options, maintaining the correct argument order and properly structured dependency injection.
Code Review: feat(skill): Improve skill generation for remote repositories✅ Overall AssessmentThis PR successfully addresses three key issues with skill generation from remote repositories. The implementation is clean, well-tested, and maintains backward compatibility. The changes significantly improve the user experience when generating skills from remote sources. 🎯 Key Improvements
💡 Suggestions for Consideration1. Improve link text in SKILL.md footerIn This would display as "from Vite" instead of the full URL twice, improving readability. 2. Extract Title Case conversion to shared utilityThe Title Case conversion logic is duplicated in both // skillUtils.ts:54-57 and :79-82
const toTitleCase = (text: string): string => {
return text
.replace(/[-_]/g, ' ')
.replace(/\b\w/g, (char) => char.toUpperCase())
.trim();
};
export const generateProjectName = (rootDirs: string[]): string => {
const primaryDir = rootDirs[0] || '.';
const dirName = path.basename(path.resolve(primaryDir));
return toTitleCase(dirName);
};
export const generateProjectNameFromUrl = (remoteUrl: string): string => {
const repoName = extractRepoName(remoteUrl);
return toTitleCase(repoName);
};🔍 Premortem AnalysisPotential risks and mitigationsRisk 1: Non-standard Git URLs
Risk 2: Directory permission issues
Risk 3: Large repository token limits
✅ Test CoverageExcellent test coverage including:
🚀 Ready to MergeThe implementation is solid and addresses real user needs. The selective copying of Great work on this improvement! The code quality is excellent and follows the project's conventions well. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1001 +/- ##
==========================================
+ Coverage 89.50% 89.83% +0.32%
==========================================
Files 120 120
Lines 9192 9234 +42
Branches 1662 1683 +21
==========================================
+ Hits 8227 8295 +68
+ Misses 965 939 -26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Fix extractRepoName to handle trailing slashes, query strings, and fragments in URLs (e.g., https://github.com/user/repo/ now works) - Extract toTitleCase helper function to reduce code duplication - Use projectName instead of sourceUrl for link text in SKILL.md footer (displays as "from [Vite](https://...)" instead of full URL twice) - Add tests for URL edge cases (trailing slash, query, fragment)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
PR Review: feat(skill): Improve skill generation for remote repositoriesI've completed a thorough review of this PR. The implementation thoughtfully addresses issues with remote repository skill generation, particularly around project name derivation, source URL attribution, and selective directory copying. Strengths
Code Quality Suggestions
Premortem AnalysisThe implementation handles potential edge cases well:
Overall AssessmentThis PR is ready to merge. The implementation successfully resolves the identified issues while maintaining backward compatibility. The selective copying approach is particularly thoughtful. Great work on this enhancement! |
Replace regex patterns that could cause polynomial time complexity with safe string methods: - /[?#].*$/ → indexOf + slice - /\/+$/ → iterative trimTrailingSlashes helper This addresses GitHub Advanced Security warnings about potential ReDoS attacks with maliciously crafted URLs containing many repetitions of '#' or '/'.
Code Review SummaryI've reviewed PR #1001 comprehensively, and this is a well-implemented enhancement to the skill generation feature for remote repositories. The changes effectively address the three main issues outlined in the PR description. ✅ Strengths
🔍 Code Quality ObservationsMinor improvements to consider
🎯 Premortem AnalysisPotential edge cases and mitigationsRisk 1: Non-standard Git URLs
Risk 2: Malformed URLs in skillSourceUrl
Risk 3: Concurrent skill generation
✅ Overall AssessmentThis PR is ready to merge. The implementation is solid, maintains backward compatibility, and follows project conventions. The selective directory copying and improved project name generation significantly enhance the user experience when working with remote repositories. Particularly commendable:
Great work on this improvement! 🚀 |
1 participant
Summary
This PR improves the
--skill-generatefeature when used with--remote:Fix project name generation: Previously, the project name was incorrectly derived from the temp directory name (e.g., "Repomix HPkbgH"). Now it correctly uses the repository name from the URL (e.g., "Vite" for
https://github.com/vitejs/vite).Add source URL to SKILL.md: For remote repositories, the SKILL.md footer now includes the source URL:
This skill was generated by [Repomix](...) from [https://github.com/vitejs/vite](...)This skill was generated by [Repomix](...)(no URL to avoid exposing personal paths)Fix
.claude/directory copy conflict: When the remote repository has its own.claude/directory (with commands, agents, etc.), only.claude/skills/is now copied to prevent conflicts.Changes
generateProjectNameFromUrl()function to extract and format project name from URLsskillSourceUrloption to pass source URL through the pipelinecopySkillOutputToCurrentDirectoryto copy only.claude/skills/Checklist
npm run testnpm run lint