Use directory copy instead of symlinks for skills on Windows#535
Use directory copy instead of symlinks for skills on Windows#535
Conversation
On Windows, creating symlinks requires Administrator privileges. Update the skills installer to copy skill directories on Windows instead of using symlinks, while keeping symlinks on Unix. The uninstall logic is also updated to handle both symlinks (for existing Unix installs) and copied directories (for Windows). Closes #533 Co-Authored-By: Sasha Varlamov <sasha@sashavarlamov.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
|
|
No AI authorship found for these commits. Please install git-ai to start tracking AI generated code in your commits. |
|
Are there unit tests covering the skill installation? If not, can you add some to verify functionality and prevent regresions? |
|
The existing tests only check that embedded skills are loaded and the skills dir path is correct. I'll add tests covering the actual install/uninstall logic including the linking and cleanup behavior. |
Co-Authored-By: Sasha Varlamov <sasha@sashavarlamov.com>
Use directory copy instead of symlinks for skills on Windows
Summary
Resolves #533. On Windows, creating symlinks requires Administrator privileges for legacy reasons. This PR updates the skills installer to copy skill directories on Windows instead of creating symlinks, removing the admin requirement. Unix behavior (symlinks) is unchanged.
Changes in
src/mdm/skills_installer.rs:create_skills_symlink→link_skill_dir: uses#[cfg(unix)]symlink /#[cfg(windows)]recursive copycopy_dir_recursive(Windows-only): recursively copies a skill directory and its contentsremove_skills_symlink→remove_skill_link: now removes both symlinks (existing Unix installs) and copied directories (Windows), so uninstall works correctly on both platformsUpdates since last revision
Added unit tests covering the skills installation and linking logic (11 new tests total):
link_skill_dir: content accessibility, replacing existing directories/files, parent directory creation, symlink verification on Unixremove_skill_link: directory removal, symlink removal, no-op on nonexistent pathstest_install_and_uninstall_skills_lifecycle: full install → re-install → uninstall → noop-uninstall lifecycleNote: These tests exercise the Unix (symlink) code path. The Windows
copy_dir_recursivepath is verified by compilation only — it cannot be exercised in the Linux/macOS test runners.Review & Testing Checklist for Human
#[cfg(windows)]code paths (copy_dir_recursive, the copy branch inlink_skill_dir) cannot be tested on Linux CI. Rungit-ai install-hookson Windows without admin privileges and confirm skills appear correctly in~/.agents/skills/and~/.claude/skills/remove_skill_linksafety: the new logic callsremove_dir_allon non-symlink directories at the skill link paths — confirm this is acceptable given the paths are always~/.agents/skills/{skill-name}or~/.claude/skills/{skill-name}Notes