From 2ed565c6671425dd9c329d2ecf1cecefb3c71f9b Mon Sep 17 00:00:00 2001 From: hatayama Date: Wed, 8 Jul 2026 18:41:09 +0900 Subject: [PATCH] Remove redundant full sync skill pruning Delete the prune pass that could only target skill names already handled by the full sync install and cleanup steps. Add coverage for disabled skills in both layouts so the intended cleanup path stays explicit. --- .../Editor/ToolSkillSynchronizerTests.cs | 51 +++++++++++++++ .../SkillSetup/SkillInstallLayout.cs | 12 ---- .../SkillSetup/SkillTargetInstaller.cs | 62 ------------------- 3 files changed, 51 insertions(+), 74 deletions(-) diff --git a/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs b/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs index 99a895f856..796de18007 100644 --- a/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs +++ b/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs @@ -315,6 +315,57 @@ await ToolSkillSynchronizer.InstallSkillFilesAtProjectRoot( Assert.That(Directory.Exists(managedSkillsRoot), Is.False); } + [Test] + public async Task InstallSkillFilesAtProjectRoot_WhenDisabledSkillExistsInBothLayouts_RemovesBothLayouts() + { + // Tests that full sync removes disabled skills through the scope cleanup before installing enabled skills. + string temporaryRoot = CreateTemporaryProjectRoot(); + CreateFakeSourceSkill( + temporaryRoot, + "uloop-enabled-skill", + "EnabledTool", + "reference.md", + "enabled-reference"); + CreateFakeSourceSkill( + temporaryRoot, + "uloop-disabled-skill", + "DisabledTool", + "reference.md", + "disabled-reference"); + + string targetRoot = Path.Combine(temporaryRoot, ".claude"); + string skillsRoot = Path.Combine(targetRoot, SkillInstallLayout.SkillsDirName); + string managedSkillsRoot = Path.Combine( + skillsRoot, + SkillInstallLayout.ManagedSkillsDirName); + string flatDisabledSkillDir = Path.Combine(skillsRoot, "uloop-disabled-skill"); + string groupedDisabledSkillDir = Path.Combine(managedSkillsRoot, "uloop-disabled-skill"); + WriteSkillFile(flatDisabledSkillDir, "---\nname: uloop-disabled-skill\n---\n"); + WriteSkillFile(groupedDisabledSkillDir, "---\nname: uloop-disabled-skill\n---\n"); + + ToolSkillSynchronizer.SkillTargetInfo target = new( + "Claude Code", + ".claude", + "--claude", + hasSkillsDirectory: true, + hasExistingSkills: true); + + ToolSkillSynchronizer.SkillInstallResult result = + await ToolSkillSynchronizer.InstallSkillFilesAtProjectRoot( + temporaryRoot, + new[] { target }, + groupSkillsUnderUnityCliLoop: false, + disabledTools: new[] { "disabled-skill" }, + ct: CancellationToken.None); + + string enabledSkillDir = Path.Combine(skillsRoot, "uloop-enabled-skill"); + + Assert.That(result.IsSuccessful, Is.True); + Assert.That(File.Exists(Path.Combine(enabledSkillDir, SkillInstallLayout.SkillFileName)), Is.True); + Assert.That(Directory.Exists(flatDisabledSkillDir), Is.False); + Assert.That(Directory.Exists(groupedDisabledSkillDir), Is.False); + } + [Test] public async Task InstallSkillFilesAtProjectRoot_WhenFlatLayoutRequested_InstallsProjectLocalCustomSkills() { diff --git a/Packages/src/Editor/Infrastructure/SkillSetup/SkillInstallLayout.cs b/Packages/src/Editor/Infrastructure/SkillSetup/SkillInstallLayout.cs index a19f9b92e7..17c38d0821 100644 --- a/Packages/src/Editor/Infrastructure/SkillSetup/SkillInstallLayout.cs +++ b/Packages/src/Editor/Infrastructure/SkillSetup/SkillInstallLayout.cs @@ -332,18 +332,6 @@ internal static string GetInstalledSkillDirectoryPathForLayout( return GetInstalledSkillDirectoryPath(targetRoot, skillName, groupSkillsUnderUnityCliLoop); } - internal static IEnumerable EnumerateInstalledSkillDirectoryNamesForLayout( - string targetRoot, - bool groupSkillsUnderUnityCliLoop) - { - IEnumerable installedSkillDirectories = groupSkillsUnderUnityCliLoop - ? EnumerateManagedSkillDirectories(targetRoot) - : EnumerateLegacyManagedSkillDirectories(targetRoot); - return installedSkillDirectories - .Select(Path.GetFileName) - .Where(name => !string.IsNullOrEmpty(name)); - } - private static IEnumerable EnumerateManagedSkillDirectories(string targetRoot) { string managedSkillsRoot = GetManagedSkillsRoot(targetRoot); diff --git a/Packages/src/Editor/Infrastructure/SkillSetup/SkillTargetInstaller.cs b/Packages/src/Editor/Infrastructure/SkillSetup/SkillTargetInstaller.cs index c79a71bec3..f31da5c90a 100644 --- a/Packages/src/Editor/Infrastructure/SkillSetup/SkillTargetInstaller.cs +++ b/Packages/src/Editor/Infrastructure/SkillSetup/SkillTargetInstaller.cs @@ -1,4 +1,3 @@ -using System; using System.Collections.Generic; using System.IO; using System.Linq; @@ -96,27 +95,6 @@ private static void InstallSkillsForTargetCore( DeleteSkillDirectoryIfExists(targetRoot, skill.Name, !groupSkillsUnderUnityCliLoop, ct); } - if (syncScope == SkillSyncScope.FullSync) - { - HashSet managedSkillNames = GetManagedSkillNames(skills, disabledSkills); - DeleteUnexpectedInstalledSkillDirectories( - targetRoot, - skills.Select(skill => skill.Name), - managedSkillNames, - groupSkillsUnderUnityCliLoop, - ct); - - if (!groupSkillsUnderUnityCliLoop) - { - DeleteUnexpectedInstalledSkillDirectories( - targetRoot, - skills.Select(skill => skill.Name), - managedSkillNames, - groupSkillsUnderUnityCliLoop: true, - ct); - } - } - if (!groupSkillsUnderUnityCliLoop) { DeleteEmptyManagedSkillsParentDirectoryIfNeeded( @@ -126,18 +104,6 @@ private static void InstallSkillsForTargetCore( } } - private static HashSet GetManagedSkillNames( - IEnumerable skills, - IEnumerable disabledSkills) - { - HashSet managedSkillNames = new( - skills.Select(skill => skill.Name), - StringComparer.Ordinal); - managedSkillNames.UnionWith(disabledSkills.Select(skill => skill.Name)); - managedSkillNames.UnionWith(DeprecatedSkillNames); - return managedSkillNames; - } - internal static void DeleteSkillDirectoryIfExists( string targetRoot, string skillName, @@ -158,34 +124,6 @@ internal static void DeleteSkillDirectoryIfExists( DeleteEmptyManagedSkillsParentDirectoryIfNeeded(targetRoot, groupSkillsUnderUnityCliLoop, ct); } - private static void DeleteUnexpectedInstalledSkillDirectories( - string targetRoot, - IEnumerable expectedSkillNames, - IEnumerable removableSkillNames, - bool groupSkillsUnderUnityCliLoop, - CancellationToken ct) - { - HashSet expectedSkillNameSet = new(expectedSkillNames, StringComparer.Ordinal); - HashSet removableSkillNameSet = new(removableSkillNames, StringComparer.Ordinal); - foreach (string installedSkillName in SkillInstallLayout.EnumerateInstalledSkillDirectoryNamesForLayout( - targetRoot, - groupSkillsUnderUnityCliLoop)) - { - ct.ThrowIfCancellationRequested(); - if (expectedSkillNameSet.Contains(installedSkillName)) - { - continue; - } - - if (!removableSkillNameSet.Contains(installedSkillName)) - { - continue; - } - - DeleteSkillDirectoryIfExists(targetRoot, installedSkillName, groupSkillsUnderUnityCliLoop, ct); - } - } - private static void DeleteDeprecatedSkillDirectories( string targetRoot, bool groupSkillsUnderUnityCliLoop,