diff --git a/desktop/src-tauri/src/managed_agents/nest.rs b/desktop/src-tauri/src/managed_agents/nest.rs index a57676f0a9..f270ffefe7 100644 --- a/desktop/src-tauri/src/managed_agents/nest.rs +++ b/desktop/src-tauri/src/managed_agents/nest.rs @@ -20,6 +20,7 @@ use tauri::{AppHandle, Manager}; use crate::managed_agents::discovery::known_skill_dirs; #[cfg(unix)] use crate::util::create_symlink; +use crate::util::{create_dir_link, remove_dir_link}; /// Subdirectories created inside the nest. /// `REPOS` is intentionally absent: it is provisioned by @@ -282,10 +283,29 @@ pub fn ensure_nest_at(root: &Path) -> Result<(), String> { Ok(()) } +/// Link target for `/buzz-cli`, given the nest `root`. +/// +/// Unix uses a target relative to the link so the nest stays movable. Windows +/// cannot: when symlink creation is denied, [`create_dir_link`] falls back to a +/// junction, and junctions store only absolute targets. Pinning to `root` is +/// the cost of not requiring Developer Mode or elevation. +fn skill_link_target(root: &Path, skill_dir: &str) -> PathBuf { + #[cfg(unix)] + { + let _ = root; + let depth = Path::new(skill_dir).components().count(); + PathBuf::from(format!("{}{CANONICAL_SKILL_DIR}", "../".repeat(depth))) + } + #[cfg(not(unix))] + { + let _ = skill_dir; + root.join(CANONICAL_SKILL_DIR) + } +} + /// Create harness-specific skill symlinks for each known provider. /// Idempotent: skips any path where `symlink_metadata` succeeds — real /// directories, valid symlinks, and dangling symlinks are all left alone. -#[cfg(unix)] fn ensure_skill_symlinks(root: &Path) -> Result<(), String> { for skill_dir in known_skill_dirs() { let parent = root.join(skill_dir); @@ -294,20 +314,13 @@ fn ensure_skill_symlinks(root: &Path) -> Result<(), String> { if link.symlink_metadata().is_ok() { continue; // symlink or real path exists — skip } - let depth = std::path::Path::new(skill_dir).components().count(); - let prefix = "../".repeat(depth); - let target = format!("{prefix}{CANONICAL_SKILL_DIR}"); - create_symlink(std::path::Path::new(&target), &link) - .map_err(|e| format!("symlink {} → {}: {e}", link.display(), target))?; + let target = skill_link_target(root, skill_dir); + create_dir_link(&target, &link) + .map_err(|e| format!("skill link {} → {}: {e}", link.display(), target.display()))?; } Ok(()) } -#[cfg(not(unix))] -fn ensure_skill_symlinks(_root: &Path) -> Result<(), String> { - Ok(()) -} - /// Returns the `~/.local/bin` link name for the bundled CLI. /// /// Dev builds (`is_dev = true`) use `"buzz-dev"` so that a running DMG and a @@ -490,27 +503,26 @@ fn refresh_skill_md_if_stale(root: &Path) -> Result<(), String> { .map_err(|e| format!("remove {}: {e}", old_skill_dir.display()))?; } - // Create/replace the .claude/skills/buzz-cli symlink. - #[cfg(unix)] + // Create/replace the .claude/skills/buzz-cli symlink. The old real + // directory is removed above on every platform, so this must run on every + // platform too or Windows is left with no link at all until the next boot. { let claude_skills_dir = root.join(".claude/skills"); fs::create_dir_all(&claude_skills_dir) .map_err(|e| format!("create {}: {e}", claude_skills_dir.display()))?; - let symlink_path = root.join(".claude/skills/buzz-cli"); + let symlink_path = claude_skills_dir.join("buzz-cli"); // Remove any stale symlink before (re)creating. let symlink_exists = symlink_path .symlink_metadata() .map(|m| m.file_type().is_symlink()) .unwrap_or(false); if symlink_exists { - fs::remove_file(&symlink_path) + remove_dir_link(&symlink_path) .map_err(|e| format!("remove symlink {}: {e}", symlink_path.display()))?; } - create_symlink( - std::path::Path::new("../../.agents/skills/buzz-cli"), - &symlink_path, - ) - .map_err(|e| format!("symlink {}: {e}", symlink_path.display()))?; + let target = skill_link_target(root, ".claude/skills"); + create_dir_link(&target, &symlink_path) + .map_err(|e| format!("symlink {}: {e}", symlink_path.display()))?; } fs::write(&version_path, format!("{NEST_SKILL_VERSION}\n")) diff --git a/desktop/src-tauri/src/managed_agents/nest/tests.rs b/desktop/src-tauri/src/managed_agents/nest/tests.rs index cbef171f6f..4c431b6c61 100644 --- a/desktop/src-tauri/src/managed_agents/nest/tests.rs +++ b/desktop/src-tauri/src/managed_agents/nest/tests.rs @@ -256,7 +256,6 @@ fn ensure_nest_migrates_old_skill_dir() { ); } -#[cfg(unix)] #[test] fn ensure_skill_symlinks_are_idempotent() { let tmp = tempfile::tempdir().unwrap(); @@ -264,15 +263,24 @@ fn ensure_skill_symlinks_are_idempotent() { ensure_nest_at(&root).unwrap(); // Second call should succeed without errors. ensure_nest_at(&root).unwrap(); - // All symlinks still valid and point to relative targets. for dir in [".goose/skills", ".claude/skills", ".codex/skills"] { let link = root.join(dir).join("buzz-cli"); assert!(link.symlink_metadata().unwrap().file_type().is_symlink()); assert!( link.join("SKILL.md").exists(), - "symlink at {dir}/buzz-cli should resolve to dir with SKILL.md" + "link at {dir}/buzz-cli should resolve to dir with SKILL.md" ); - let target = fs::read_link(&link).unwrap(); + } +} + +#[cfg(unix)] +#[test] +fn ensure_skill_symlinks_use_relative_targets_on_unix() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + ensure_nest_at(&root).unwrap(); + for dir in [".goose/skills", ".claude/skills", ".codex/skills"] { + let target = fs::read_link(root.join(dir).join("buzz-cli")).unwrap(); assert_eq!( target.to_str().unwrap(), format!("../../{CANONICAL_SKILL_DIR}"), @@ -281,7 +289,27 @@ fn ensure_skill_symlinks_are_idempotent() { } } -#[cfg(unix)] +/// Windows junctions — the fallback when symlink creation is denied — store +/// only absolute targets, so the nest is not relocatable there. +#[cfg(windows)] +#[test] +fn ensure_skill_symlinks_use_absolute_targets_on_windows() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path().join(".buzz"); + ensure_nest_at(&root).unwrap(); + // Windows stores link targets in verbatim (`\\?\C:\…`) form, which never + // compares equal to the path passed in — canonicalize both sides. + let canonical = root.join(CANONICAL_SKILL_DIR).canonicalize().unwrap(); + for dir in [".goose/skills", ".claude/skills", ".codex/skills"] { + let link = root.join(dir).join("buzz-cli"); + let stored = fs::read_link(&link).unwrap().canonicalize().unwrap(); + assert_eq!( + stored, canonical, + "link at {dir}/buzz-cli should resolve to the canonical skill dir" + ); + } +} + #[test] fn ensure_skill_symlinks_skips_existing_path_during_initial_pass() { // ensure_skill_symlinks skips any path where symlink_metadata succeeds. @@ -316,28 +344,32 @@ fn ensure_skill_symlinks_skips_existing_path_during_initial_pass() { ); } -#[cfg(unix)] #[test] fn ensure_skill_symlinks_skip_dangling_symlink() { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path().join(".buzz"); - // Pre-create a dangling symlink where the .codex link would go. + // Pre-create a dangling link where the .codex link would go. let codex_skills = root.join(".codex/skills"); fs::create_dir_all(&codex_skills).unwrap(); let dangling = codex_skills.join("buzz-cli"); - std::os::unix::fs::symlink("/nonexistent/target", &dangling).unwrap(); + let missing = tmp.path().join("nonexistent-target"); + // A junction cannot be created against a missing target, so make the + // target vanish after linking rather than before. + fs::create_dir_all(&missing).unwrap(); + crate::util::create_dir_link(&missing, &dangling).unwrap(); + fs::remove_dir(&missing).unwrap(); ensure_nest_at(&root).unwrap(); - // Dangling symlink should be left alone (not clobbered). + // Dangling link should be left alone (not clobbered). assert!(dangling .symlink_metadata() .unwrap() .file_type() .is_symlink()); - assert_eq!( - fs::read_link(&dangling).unwrap().to_str().unwrap(), - "/nonexistent/target" + assert!( + !dangling.join("SKILL.md").exists(), + "dangling link should not have been repointed at the canonical skill dir" ); } diff --git a/desktop/src-tauri/src/util.rs b/desktop/src-tauri/src/util.rs index 7fa4dc6768..b98db948eb 100644 --- a/desktop/src-tauri/src/util.rs +++ b/desktop/src-tauri/src/util.rs @@ -77,6 +77,69 @@ pub(crate) fn symlink_points_to(link: &std::path::Path, target: &std::path::Path .unwrap_or(false) } +/// Link a directory at `link` so it resolves to `target`. +/// +/// Unix uses a plain symlink. Windows prefers a directory symlink and falls +/// back to a junction, which needs neither Developer Mode nor elevation — +/// without the fallback this fails for most users. Junctions only address +/// local volumes, so the symlink attempt comes first to keep UNC targets +/// working. +#[cfg(unix)] +pub(crate) fn create_dir_link( + target: &std::path::Path, + link: &std::path::Path, +) -> std::io::Result<()> { + std::os::unix::fs::symlink(target, link) +} + +#[cfg(windows)] +pub(crate) fn create_dir_link( + target: &std::path::Path, + link: &std::path::Path, +) -> std::io::Result<()> { + use std::os::windows::process::CommandExt; + + let symlink_error = match std::os::windows::fs::symlink_dir(target, link) { + Ok(()) => return Ok(()), + Err(error) => error, + }; + + const CREATE_NO_WINDOW: u32 = 0x0800_0000; + // `raw_arg` because cmd.exe re-parses its command line with rules the + // standard argument escaping does not match; paths cannot contain `"`, + // so quoting them here is sufficient. + let output = std::process::Command::new("cmd") + .arg("/C") + .raw_arg(format!( + "mklink /J \"{}\" \"{}\"", + link.display(), + target.display() + )) + .creation_flags(CREATE_NO_WINDOW) + .output()?; + if output.status.success() { + return Ok(()); + } + Err(std::io::Error::other(format!( + "symlink failed ({symlink_error}) and junction fallback failed: {}", + String::from_utf8_lossy(&output.stderr).trim() + ))) +} + +/// Remove a directory link created by [`create_dir_link`]. +/// +/// Windows directory symlinks and junctions are directory entries, so +/// `remove_file` refuses them; `remove_dir` unlinks without following. +#[cfg(unix)] +pub(crate) fn remove_dir_link(link: &std::path::Path) -> std::io::Result<()> { + std::fs::remove_file(link) +} + +#[cfg(windows)] +pub(crate) fn remove_dir_link(link: &std::path::Path) -> std::io::Result<()> { + std::fs::remove_dir(link) +} + /// Compute a collision-safe backup path for `dst`. /// /// The candidate is `/.bak.`. If that