diff --git a/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs b/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs index c8b6368b4..19258acb8 100644 --- a/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs +++ b/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs @@ -8,8 +8,11 @@ namespace io.github.hatayama.uLoopMCP [TestFixture] public class ToolSkillSynchronizerTests { + private const string SkillsDirName = "skills"; + private string _projectRoot; private string[] _nonExistentDirsBefore; + private string[] _temporaryRoots; [SetUp] public void SetUp() @@ -19,6 +22,7 @@ public void SetUp() _nonExistentDirsBefore = ToolSkillSynchronizer.SkillTargetDirs .Where(dir => !Directory.Exists(Path.Combine(_projectRoot, dir))) .ToArray(); + _temporaryRoots = new string[0]; } [TearDown] @@ -31,7 +35,7 @@ public void TearDown() if (Directory.Exists(fullPath)) { // Only delete if it was created by this test (didn't exist before) - string skillsPath = Path.Combine(fullPath, "skills"); + string skillsPath = Path.Combine(fullPath, SkillsDirName); if (Directory.Exists(skillsPath)) { Directory.Delete(skillsPath, true); @@ -43,6 +47,14 @@ public void TearDown() } } } + + foreach (string temporaryRoot in _temporaryRoots) + { + if (Directory.Exists(temporaryRoot)) + { + Directory.Delete(temporaryRoot, true); + } + } } [Test] @@ -64,6 +76,54 @@ public async Task InstallSkillFiles_DoesNotCreateNonExistentTargetDirectories() } } + [Test] + public void DetectTargets_DoesNotIncludeTargetsWithOnlyParentDirectory() + { + // Arrange + string temporaryRoot = CreateTemporaryProjectRoot(); + foreach (string dir in ToolSkillSynchronizer.SkillTargetDirs) + { + Directory.CreateDirectory(Path.Combine(temporaryRoot, dir)); + } + + // Act + string[] detectedTargetDirs = ToolSkillSynchronizer.DetectTargets(temporaryRoot, requireSkillsDirectory: true) + .Select(target => target.DirName) + .ToArray(); + + // Assert + foreach (string dir in ToolSkillSynchronizer.SkillTargetDirs) + { + Assert.IsFalse(detectedTargetDirs.Contains(dir), + $"Target '{dir}' should not be detected when only the parent directory exists"); + } + } + + [Test] + public void DetectTargets_IncludesTargetsWhenSkillsDirectoryExists() + { + // Arrange + string temporaryRoot = CreateTemporaryProjectRoot(); + foreach (string dir in ToolSkillSynchronizer.SkillTargetDirs) + { + Directory.CreateDirectory(Path.Combine(temporaryRoot, dir, SkillsDirName)); + } + + // Act + ToolSkillSynchronizer.SkillTargetInfo[] detectedTargets = ToolSkillSynchronizer.DetectTargets(temporaryRoot, requireSkillsDirectory: true) + .ToArray(); + + // Assert + Assert.AreEqual(ToolSkillSynchronizer.SkillTargetDirs.Length, detectedTargets.Length, + "Targets with a skills directory should be detected"); + + foreach (ToolSkillSynchronizer.SkillTargetInfo target in detectedTargets) + { + Assert.IsFalse(target.HasExistingSkills, + $"Target '{target.DirName}' should not be treated as already installed when skills directory is empty"); + } + } + [Test] public void RemoveSkillFiles_DoesNotCreateNonExistentTargetDirectories() { @@ -99,5 +159,16 @@ public void IsSkillInstalled_DoesNotCreateNonExistentTargetDirectories() $"Directory '{dir}' should not be created by IsSkillInstalled"); } } + + private string CreateTemporaryProjectRoot() + { + string temporaryRoot = Path.Combine( + Path.GetTempPath(), + "ToolSkillSynchronizerTests", + System.Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(temporaryRoot); + _temporaryRoots = _temporaryRoots.Append(temporaryRoot).ToArray(); + return temporaryRoot; + } } } diff --git a/Packages/src/Editor/Config/ToolSkillSynchronizer.cs b/Packages/src/Editor/Config/ToolSkillSynchronizer.cs index f39bfdd47..424324b55 100644 --- a/Packages/src/Editor/Config/ToolSkillSynchronizer.cs +++ b/Packages/src/Editor/Config/ToolSkillSynchronizer.cs @@ -17,6 +17,9 @@ namespace io.github.hatayama.uLoopMCP /// public static class ToolSkillSynchronizer { + private const string SkillsDirName = "skills"; + private const string SkillFileName = "SKILL.md"; + public readonly struct SkillTargetDefinition { public readonly string DirName; @@ -78,7 +81,7 @@ public static void RemoveSkillFiles(string toolName) foreach (string targetDir in SkillTargetDirs) { - string skillsRoot = Path.Combine(projectRoot, targetDir, "skills"); + string skillsRoot = Path.Combine(projectRoot, targetDir, SkillsDirName); if (!Directory.Exists(skillsRoot)) { continue; @@ -103,7 +106,7 @@ public static bool IsSkillInstalled(string toolName) foreach (string targetDir in SkillTargetDirs) { - string skillsRoot = Path.Combine(projectRoot, targetDir, "skills"); + string skillsRoot = Path.Combine(projectRoot, targetDir, SkillsDirName); if (!Directory.Exists(skillsRoot)) { continue; @@ -122,28 +125,42 @@ public static bool IsSkillInstalled(string toolName) return false; } - /// - /// Checks parent directories (.claude/, .agents/, etc.), not skills/ subdirectories. - /// public static List DetectTargets() + { + return DetectTargets(requireSkillsDirectory: false); + } + + internal static List DetectTargets(bool requireSkillsDirectory) { string projectRoot = UnityMcpPathResolver.GetProjectRoot(); Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); + return DetectTargets(projectRoot, requireSkillsDirectory); + } + + internal static List DetectTargets(string projectRoot, bool requireSkillsDirectory) + { + Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty"); + List targets = new(); foreach (SkillTargetDefinition target in SkillTargets) { - string parentDir = Path.Combine(projectRoot, target.DirName); - if (!Directory.Exists(parentDir)) + string targetRoot = Path.Combine(projectRoot, target.DirName); + if (!Directory.Exists(targetRoot)) + { + continue; + } + + string skillsRoot = Path.Combine(targetRoot, SkillsDirName); + if (requireSkillsDirectory && !Directory.Exists(skillsRoot)) { continue; } - string skillsRoot = Path.Combine(parentDir, "skills"); bool hasULoopSkills = Directory.Exists(skillsRoot) && Directory.EnumerateDirectories(skillsRoot, CliConstants.SKILL_DIR_GLOB) - .Any(skillDir => File.Exists(Path.Combine(skillDir, "SKILL.md"))); + .Any(skillDir => File.Exists(Path.Combine(skillDir, SkillFileName))); targets.Add(new SkillTargetInfo(target.DisplayName, target.DirName, hasULoopSkills)); } @@ -151,12 +168,11 @@ public static List DetectTargets() } /// - /// Re-install skills for all detected targets. - /// Checks parent directories (.claude/, .agents/, etc.) to determine targets. + /// Re-installs skills only for targets that already opted in via an existing skills directory. /// public static async Task InstallSkillFiles() { - List targets = DetectTargets(); + List targets = DetectTargets(requireSkillsDirectory: true); return await InstallSkillFiles(targets); } @@ -196,7 +212,7 @@ public static async Task InstallSkillFiles(List