[PR #535] [MERGED] Use directory copy instead of symlinks for skills on Windows #551

New issue

Closed

opened 2026-03-02 04:13:57 +03:00 by kerem · 0 comments

kerem commented

2026-03-02 04:13:57 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/git-ai-project/git-ai/pull/535
Author: @svarlamov
Created: 2/16/2026
Status: ✅ Merged
Merged: 2/17/2026
Merged by: @svarlamov

Base: main ← Head: devin/1771275494-windows-skills-copy-instead-of-symlinks

📝 Commits (2)

8072651 Use directory copy instead of symlinks for skills on Windows
70727d6 Add unit tests for skills installation and linking logic

📊 Changes

1 file changed (+206 additions, -35 deletions)

View changed files

📝 src/mdm/skills_installer.rs (+206 -35)

📄 Description

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 copy
New copy_dir_recursive (Windows-only): recursively copies a skill directory and its contents
remove_skills_symlink → remove_skill_link: now removes both symlinks (existing Unix installs) and copied directories (Windows), so uninstall works correctly on both platforms

Updates 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 Unix
remove_skill_link: directory removal, symlink removal, no-op on nonexistent paths
test_install_and_uninstall_skills_lifecycle: full install → re-install → uninstall → noop-uninstall lifecycle

Note: These tests exercise the Unix (symlink) code path. The Windows copy_dir_recursive path is verified by compilation only — it cannot be exercised in the Linux/macOS test runners.

Review & Testing Checklist for Human

Verify on a real Windows machine: the #[cfg(windows)] code paths (copy_dir_recursive, the copy branch in link_skill_dir) cannot be tested on Linux CI. Run git-ai install-hooks on Windows without admin privileges and confirm skills appear correctly in ~/.agents/skills/ and ~/.claude/skills/
Verify uninstall on Windows: run uninstall and confirm copied skill directories are cleaned up
Review remove_skill_link safety: the new logic calls remove_dir_all on 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

Requested by: @svarlamov
Link to Devin run

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/git-ai-project/git-ai/pull/535 **Author:** [@svarlamov](https://github.com/svarlamov) **Created:** 2/16/2026 **Status:** ✅ Merged **Merged:** 2/17/2026 **Merged by:** [@svarlamov](https://github.com/svarlamov) **Base:** `main` ← **Head:** `devin/1771275494-windows-skills-copy-instead-of-symlinks` --- ### 📝 Commits (2) - [`8072651`](https://github.com/git-ai-project/git-ai/commit/80726511de0baaf75d9a922bfa010b0699d7659e) Use directory copy instead of symlinks for skills on Windows - [`70727d6`](https://github.com/git-ai-project/git-ai/commit/70727d6f1efe84c3ab69ff5a461a2131f335c82e) Add unit tests for skills installation and linking logic ### 📊 Changes **1 file changed** (+206 additions, -35 deletions) <details> <summary>View changed files</summary> 📝 `src/mdm/skills_installer.rs` (+206 -35) </details> ### 📄 Description # 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 copy - New `copy_dir_recursive` (Windows-only): recursively copies a skill directory and its contents - `remove_skills_symlink` → `remove_skill_link`: now removes both symlinks (existing Unix installs) **and** copied directories (Windows), so uninstall works correctly on both platforms ## Updates 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 Unix - `remove_skill_link`: directory removal, symlink removal, no-op on nonexistent paths - `test_install_and_uninstall_skills_lifecycle`: full install → re-install → uninstall → noop-uninstall lifecycle **Note:** These tests exercise the Unix (symlink) code path. The Windows `copy_dir_recursive` path is verified by compilation only — it cannot be exercised in the Linux/macOS test runners. ## Review & Testing Checklist for Human - [ ] **Verify on a real Windows machine**: the `#[cfg(windows)]` code paths (`copy_dir_recursive`, the copy branch in `link_skill_dir`) cannot be tested on Linux CI. Run `git-ai install-hooks` on Windows **without** admin privileges and confirm skills appear correctly in `~/.agents/skills/` and `~/.claude/skills/` - [ ] **Verify uninstall on Windows**: run uninstall and confirm copied skill directories are cleaned up - [ ] **Review `remove_skill_link` safety**: the new logic calls `remove_dir_all` on 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 - Requested by: @svarlamov - [Link to Devin run](https://app.devin.ai/sessions/90ef613119674fdc943d3a87938e6d15) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>