feat(claude-skill-to-desktop): new tool to sync Claude skills to desktop apps#34
feat(claude-skill-to-desktop): new tool to sync Claude skills to desktop apps#34
Conversation
fix: guard non-dirs in findManifestPath, fix description regex, rm+cp on reinstall feat: implement interactive skill sync with clack multiselect feat: claude-skill-to-desktop tool complete — smoke tested fix: resolve two pre-existing TypeScript compilation errors - azure-devops/history-search: wiql was set to a plain object instead of calling buildCombinedQuery() when --current flag is used with assignedTo - utils/markdown: add type declaration for untyped turndown-plugin-gfm package
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ 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 |
Summary of ChangesHello @genesiscz, 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 introduces a significant new utility for managing Claude skills, enabling seamless synchronization between local development environments and the Claude Desktop application. It streamlines the process of installing and updating skills, enhancing the developer workflow for Claude skill creators. Additionally, it includes important TypeScript fixes that improve the robustness of existing Azure DevOps integration and provide better type support for markdown processing. 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. 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
|
![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 a valuable new tool for synchronizing Claude skills to the desktop application. The implementation is well-structured, utilizing appropriate libraries and a clear command-line interface. The inclusion of fixes for other parts of the codebase is also appreciated. My review highlights a few areas for improvement in the new tool, including a potential issue where built-in skills could be overwritten, along with some suggestions to enhance usability and performance. Overall, this is a solid contribution.
src/claude-skill-to-desktop/index.ts
Outdated
| const manifest = readManifest(manifestPath); | ||
| const skills = discoverLocalSkills(manifest); |
There was a problem hiding this comment.
There's a risk of overwriting built-in skills. The README.md states that skills with creatorType: "anthropic" are not overwritten, but the current implementation doesn't enforce this. If a local skill shares a name with a built-in one, this tool will overwrite the manifest entry. It would be safer to filter out these conflicting skills and notify the user.
const manifest = readManifest(manifestPath);
const allLocalSkills = discoverLocalSkills(manifest);
const builtInConflicts = allLocalSkills.filter((s) => s.installedEntry?.creatorType === "anthropic");
if (builtInConflicts.length > 0) {
p.log.warn(
`Ignoring local skills that would overwrite built-in skills: ${builtInConflicts.map((s) => s.name).join(", ")}`
);
}
const skills = allLocalSkills.filter((s) => s.installedEntry?.creatorType !== "anthropic");There was a problem hiding this comment.
@gemini-code-assist Fixed in 93fc641d — filter anthropic built-ins before multiselect (warn + skip) + throw in installSkill if name collides with a built-in entry.
There was a problem hiding this comment.
That's great to hear! I've reviewed the changes in commit 93fc641d and confirm that the issue regarding overwriting built-in skills has been effectively addressed. The implementation correctly filters out Anthropic built-in skills before the multiselect and includes a warning message, as well as an additional safeguard in installSkill to prevent overwriting them. Excellent work!
| cd /Users/Martin/Tresors/Projects/GenesisTools | ||
| bun run src/claude-skill-to-desktop/index.ts --help |
There was a problem hiding this comment.
The example command here includes a hardcoded, user-specific file path (/Users/Martin/...). To ensure the documentation is clear and useful for all contributors, it's best to use a generic placeholder.
| cd /Users/Martin/Tresors/Projects/GenesisTools | |
| bun run src/claude-skill-to-desktop/index.ts --help | |
| cd /path/to/your/project | |
| bun run src/claude-skill-to-desktop/index.ts --help |
There was a problem hiding this comment.
@gemini-code-assist Fixed in 93fc641d — replaced /Users/Martin/Tresors/Projects/GenesisTools with /path/to/your/project in the example command block.
There was a problem hiding this comment.
Great, thanks for addressing that!
src/claude-skill-to-desktop/index.ts
Outdated
| label: s.installedEntry | ||
| ? `${s.name} ${pc.dim("(already installed — will update)")}` | ||
| : s.name, | ||
| hint: s.description.length > 70 ? s.description.slice(0, 70) + "…" : s.description, |
There was a problem hiding this comment.
The interactive multiselect doesn't pre-select skills that are already installed, which makes the process of updating existing skills less convenient for the user. The implementation plan correctly identified this as a desired feature. Adding the selected property to the options will align with the plan and improve usability.
hint: s.description.length > 70 ? s.description.slice(0, 70) + "…" : s.description,
selected: !!s.installedEntry,There was a problem hiding this comment.
@gemini-code-assist Fixed in 93fc641d — added selected: !!s.installedEntry to each multiselect option to pre-select already-installed skills.
| for (const skill of toInstall) { | ||
| spinner.start(`Installing ${pc.cyan(skill.name)}...`); | ||
| try { | ||
| installSkill(skill, manifest, manifestPath); | ||
| results.push({ name: skill.name, isUpdate: !!skill.installedEntry }); | ||
| spinner.stop( | ||
| `${pc.green("✓")} ${skill.name} ${skill.installedEntry ? pc.dim("(updated)") : pc.dim("(new)")}` | ||
| ); | ||
| } catch (err) { | ||
| results.push({ name: skill.name, isUpdate: false, error: String(err) }); | ||
| spinner.stop(`${pc.red("✗")} ${skill.name}: ${String(err)}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
The manifest.json file is written to disk on every iteration of this loop. This is inefficient and can be slow if many skills are being installed at once due to repeated I/O operations. A better approach is to update the manifest object in memory within the loop and then write it to the file a single time after the loop completes.
This would involve a small refactoring:
- In
lib.ts, modifyinstallSkill()to only handle file copying and in-memory manifest updates, without writing to disk. - In
lib.ts, add a newwriteManifest()function to handle the file-writing logic. - In this file (
index.ts), call the newwriteManifest()function once after the installation loop if any skills were successfully installed.
Here is an example of how the code could be structured:
In src/claude-skill-to-desktop/lib.ts:
// Modify installSkill to remove file writing
export function installSkill(skill: LocalSkill, manifest: Manifest, manifestPath: string): SkillEntry {
// ... (file copying logic remains the same) ...
const idx = manifest.skills.findIndex((s) => s.name === skill.name);
const entry: SkillEntry = {
// ... (entry creation logic remains the same) ...
};
if (idx >= 0) {
manifest.skills[idx] = entry;
} else {
manifest.skills.push(entry);
}
return entry;
}
// Add new function to write manifest
export function writeManifest(manifest: Manifest, manifestPath: string): void {
manifest.lastUpdated = Date.now();
writeFileSync(manifestPath, JSON.stringify(manifest, null, 2));
}In src/claude-skill-to-desktop/index.ts (this file):
// ... (after the for loop)
if (succeeded.length > 0) {
spinner.start("Updating manifest...");
try {
writeManifest(manifest, manifestPath);
spinner.stop("Manifest updated.");
} catch (err) {
spinner.stop("Failed to write manifest.");
p.log.error(String(err));
}
}
// ... (outro)There was a problem hiding this comment.
@gemini-code-assist Fixed in 93fc641d — extracted writeManifest(), installSkill() now only updates the in-memory manifest, and a single writeManifest() call is made after the loop with spinner feedback.
There was a problem hiding this comment.
That's great to hear! I'm glad the suggestion was helpful and that the efficiency improvement has been implemented. Thank you for addressing it.
There was a problem hiding this comment.
Pull request overview
Adds a new CLI tool to sync Claude Code skills (~/.claude/skills) into the Claude Desktop app’s local skills registry on macOS, plus a couple of TypeScript fixes elsewhere.
Changes:
- Introduces
claude-skill-to-desktop(CLI + library + README) with interactive multiselect and install/reinstall behavior. - Fixes
azure-devops/history-searchWIQL query construction to callbuildCombinedQuery(...). - Adds a local
.d.tsdeclaration forturndown-plugin-gfmto satisfy TypeScript.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/markdown/turndown-plugin-gfm.d.ts | Adds missing typings for turndown-plugin-gfm used by markdown utilities. |
| src/claude-skill-to-desktop/lib.ts | Implements discovery of desktop manifest, local skill discovery, and install (copy + manifest update). |
| src/claude-skill-to-desktop/index.ts | Implements the CLI flow (list, all, interactive multiselect) and install loop. |
| src/claude-skill-to-desktop/README.md | Documents usage and behavior notes for the new tool. |
| src/azure-devops/commands/history-search.ts | Fixes incorrect WIQL value assignment to use buildCombinedQuery(...). |
| docs/plans/2026-02-17-claude-skill-to-desktop.md | Adds an implementation plan document for the new tool. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/claude-skill-to-desktop/index.ts
Outdated
| options: skills.map((s) => ({ | ||
| value: s, | ||
| label: s.installedEntry | ||
| ? `${s.name} ${pc.dim("(already installed — will update)")}` | ||
| : s.name, | ||
| hint: s.description.length > 70 ? s.description.slice(0, 70) + "…" : s.description, | ||
| })), |
There was a problem hiding this comment.
The multiselect treats any existing manifest entry as "already installed — will update". If an installed entry is an Anthropic built-in (creatorType "anthropic"), the tool should not offer it as updatable (or should clearly mark it as blocked) to match the documented behavior and avoid confusing UX.
| options: skills.map((s) => ({ | |
| value: s, | |
| label: s.installedEntry | |
| ? `${s.name} ${pc.dim("(already installed — will update)")}` | |
| : s.name, | |
| hint: s.description.length > 70 ? s.description.slice(0, 70) + "…" : s.description, | |
| })), | |
| options: skills.map((s) => { | |
| const isAnthropicBuiltIn = s.installedEntry?.creatorType === "anthropic"; | |
| const label = s.installedEntry | |
| ? isAnthropicBuiltIn | |
| ? `${s.name} ${pc.dim("(built-in — cannot update)")}` | |
| : `${s.name} ${pc.dim("(already installed — will update)")}` | |
| : s.name; | |
| const hint = | |
| s.description.length > 70 ? s.description.slice(0, 70) + "…" : s.description; | |
| return { | |
| value: s, | |
| label, | |
| hint, | |
| disabled: isAnthropicBuiltIn ? "Built-in skills cannot be updated" : false, | |
| }; | |
| }), |
| const skillsDir = join(manifestPath, "..", "skills"); | ||
| const destPath = join(skillsDir, skill.name); | ||
| if (existsSync(destPath)) rmSync(destPath, { recursive: true }); | ||
| mkdirSync(destPath, { recursive: true }); | ||
| cpSync(skill.sourcePath, destPath, { recursive: true }); |
There was a problem hiding this comment.
skill.name comes from SKILL.md frontmatter and is used directly in join(skillsDir, skill.name). A crafted name like ../../something or an absolute path could escape the intended skills directory, leading to unintended deletes/copies via rmSync/cpSync and writing a misleading manifest entry. Validate/sanitize the skill name before using it as a path segment (e.g., reject path separators, .., and absolute paths; or resolve and assert the destination stays within skillsDir).
There was a problem hiding this comment.
Fixed in 93fc641d — added path traversal guard using resolve() + startsWith(resolvedSkillsDir + sep) before rmSync/cpSync in installSkill().
src/claude-skill-to-desktop/lib.ts
Outdated
| const idx = manifest.skills.findIndex((s) => s.name === skill.name); | ||
| const entry: SkillEntry = { | ||
| skillId: idx >= 0 ? manifest.skills[idx].skillId : generateSkillId(), | ||
| name: skill.name, | ||
| description: skill.description, | ||
| creatorType: "user", | ||
| updatedAt: new Date().toISOString(), | ||
| enabled: true, | ||
| }; | ||
| if (idx >= 0) { | ||
| manifest.skills[idx] = entry; | ||
| } else { | ||
| manifest.skills.push(entry); | ||
| } |
There was a problem hiding this comment.
This overwrites any existing manifest entry that matches by name, including entries with creatorType: "anthropic". That conflicts with the README note and can clobber built-in skills if a user skill name collides. Before replacing manifest.skills[idx] (and before deleting/copying the skill folder), detect creatorType === "anthropic" and skip/throw with a clear message so built-ins are never overwritten.
There was a problem hiding this comment.
Fixed in 93fc641d — installSkill() now throws if an existing manifest entry has creatorType === "anthropic" before any file operations or manifest modification.
| // Copy directory | ||
| const destPath = join(skillsDir, skill.name); |
There was a problem hiding this comment.
The planned installSkill implementation uses skill.name from SKILL.md frontmatter directly in join(skillsDir, skill.name) to build destPath for copying files, which allows names containing ../ segments or absolute paths to escape the intended Claude Desktop skills directory. A malicious or untrusted skill definition could set a crafted name so that running this tool (--all or interactive) writes into arbitrary locations in the user’s filesystem, potentially overwriting important files. To prevent this, ensure skill.name is sanitized to a single safe path segment (e.g., derived from dirName or validated to reject path separators and ..) before using it in filesystem paths.
| // Copy directory | |
| const destPath = join(skillsDir, skill.name); | |
| // Copy directory (use dirName to ensure a safe, single path segment) | |
| const destPath = join(skillsDir, skill.dirName); |
There was a problem hiding this comment.
Fixed in 93fc641d — plan doc updated to use skill.dirName with a note about path validation to prevent traversal.
- Guard against overwriting anthropic built-in skills (filter + lib-level throw) - Pre-select already-installed skills in multiselect - Write manifest once after loop instead of per iteration - Sanitize skill.name against path traversal - Fix hardcoded user path in docs
Summary
claude-skill-to-desktoptool that syncs Claude Code skills to desktop Claude app@clack/promptsto pick which skills to installfindManifestPath, fixes description regex, uses rm+cp on reinstallazure-devops/history-search:wiqlwas set to a plain object instead of callingbuildCombinedQuery()utils/markdown: adds type declaration for untypedturndown-plugin-gfmpackageTest plan
tools claude-skill-to-desktop— interactive skill picker launchesbunx tsgo --noEmit— zero compilation errors