fix: copy skills dir to plugin cache on sync - #623
Conversation
The skills symlink at plugins/genie/skills doesn't survive npm packaging. syncPlugin() now copies skills/ from the package root to the cache dir as a fallback.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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, 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 resolves an issue where Genie skills were not accessible after npm packaging due to symlinks failing to persist. It introduces a new step in the Highlights
Changelog
Activity
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
|
There was a problem hiding this comment.
Code Review
This pull request addresses an issue where the skills directory is not available after an npm installation by copying it to the plugin cache during the syncPlugin process. The logic is sound. I've added one comment suggesting the use of the standard fs.cpSync instead of the custom copyDirSync for improved robustness and to correctly handle potential symbolic links within the skills directory.
| if (existsSync(skillsSrc) && !existsSync(join(cacheDir, 'skills'))) { | ||
| copyDirSync(skillsSrc, join(cacheDir, 'skills')); | ||
| } |
There was a problem hiding this comment.
The custom copyDirSync function used here does not correctly handle symbolic links to files. It copies the symlink itself rather than the file it points to, which can result in broken links in the destination cacheDir.
Since your project requires Node.js >= 18.0.0, I recommend using the built-in fs.cpSync function. It is more robust, correctly handles symbolic links, and makes the !existsSync check redundant as it can create the destination directory.
Here is how you could implement it (you will need to import cpSync from node:fs):
if (existsSync(skillsSrc)) {
cpSync(skillsSrc, join(cacheDir, 'skills'), { recursive: true });
}Using cpSync would make this fix more reliable. Consider also replacing the other usage of copyDirSync in this file for consistency and to improve overall code health.
|
Wrong diagnosis — other skills work fine from the same path. |
Summary
Skills symlink at
plugins/genie/skills -> ../../skillsdoesn't survive npm packaging.syncPlugin()now copiesskills/from the package root to the cache dir so CC can find/genieand all other skills.+6 lines in
update.ts. Symlink preserved for dev.Test plan
genie update→/genieskill available in new CC session