diff --git a/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs b/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs index c83523b555..e36625601b 100644 --- a/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs +++ b/Assets/Tests/Editor/ToolSkillSynchronizerTests.cs @@ -1638,6 +1638,51 @@ await V3MigrationSkillInstaller.RemoveSpecificSkillFilesAtProjectRoot( Assert.That(Directory.Exists(unrelatedSkillDir), Is.True); } + [Test] + public async Task RemoveSpecificSkillFilesAtProjectRoot_WithMultipleTargets_ReportsAllTargetsSucceeded() + { + // Tests that migration skill removal reports one successful result per requested target. + string temporaryRoot = CreateTemporaryProjectRoot(); + ToolSkillSynchronizer.SkillTargetInfo codexTarget = new( + "Codex CLI", + ".codex", + "--codex", + hasSkillsDirectory: true, + hasExistingSkills: false); + ToolSkillSynchronizer.SkillTargetInfo claudeTarget = new( + "Claude Code", + ".claude", + "--claude", + hasSkillsDirectory: true, + hasExistingSkills: false); + string codexSkillDir = Path.Combine( + temporaryRoot, + ".codex", + SkillInstallLayout.SkillsDirName, + CliConstants.V3_CLI_INVOCATION_MIGRATION_SKILL_NAME); + string claudeSkillDir = Path.Combine( + temporaryRoot, + ".claude", + SkillInstallLayout.SkillsDirName, + CliConstants.V3_CLI_INVOCATION_MIGRATION_SKILL_NAME); + WriteSkillFile(codexSkillDir, "---\nname: v3-cli-invocation-migration\n---\n"); + WriteSkillFile(claudeSkillDir, "---\nname: v3-cli-invocation-migration\n---\n"); + + ToolSkillSynchronizer.SkillInstallResult result = + await V3MigrationSkillInstaller.RemoveSpecificSkillFilesAtProjectRoot( + temporaryRoot, + new[] { codexTarget, claudeTarget }, + CliConstants.V3_CLI_INVOCATION_MIGRATION_SKILL_NAME, + groupSkillsUnderUnityCliLoop: false, + ct: CancellationToken.None); + + Assert.That(result.AttemptedTargets, Is.EqualTo(2)); + Assert.That(result.SucceededTargets, Is.EqualTo(2)); + Assert.That(result.IsSuccessful, Is.True); + Assert.That(Directory.Exists(codexSkillDir), Is.False); + Assert.That(Directory.Exists(claudeSkillDir), Is.False); + } + [Test] public async Task RemoveV3MigrationSkillFilesAtProjectRoot_WhenSkillExistsInBothLayouts_RemovesBothLayouts() { diff --git a/Packages/src/Editor/Infrastructure/SkillSetup/V3MigrationSkillInstaller.cs b/Packages/src/Editor/Infrastructure/SkillSetup/V3MigrationSkillInstaller.cs index 99515d9a0b..ad839f2dbe 100644 --- a/Packages/src/Editor/Infrastructure/SkillSetup/V3MigrationSkillInstaller.cs +++ b/Packages/src/Editor/Infrastructure/SkillSetup/V3MigrationSkillInstaller.cs @@ -72,29 +72,23 @@ internal static SkillInstallState GetV3MigrationSkillInstallStateAtProjectRoot( ct.ThrowIfCancellationRequested(); SkillInstallLayout.SkillSourceInfo skill = GetV3MigrationSkillSourceInfo(); - ToolSkillSynchronizer.SkillTargetInfo[] targetArray = targets.ToArray(); - return await Task.Run(() => - { - int succeeded = 0; - foreach (ToolSkillSynchronizer.SkillTargetInfo target in targetArray) + return await RunForTargetsAsync( + targets, + (ToolSkillSynchronizer.SkillTargetInfo target, CancellationToken targetCt) => { - ct.ThrowIfCancellationRequested(); string targetRoot = Path.Combine(projectRoot, target.DirName); SkillTargetInstaller.DeleteSkillDirectoryIfExists( targetRoot, skill.Name, groupSkillsUnderUnityCliLoop, - ct); + targetCt); SkillTargetInstaller.DeleteSkillDirectoryIfExists( targetRoot, skill.Name, !groupSkillsUnderUnityCliLoop, - ct); - succeeded++; - } - - return new ToolSkillSynchronizer.SkillInstallResult(targetArray.Length, succeeded); - }, ct); + targetCt); + }, + ct); } internal static SkillInstallState GetSkillInstallStateAtProjectRoot( @@ -129,25 +123,19 @@ internal static SkillInstallState GetSkillInstallStateAtProjectRoot( Debug.Assert(targets != null, "targets must not be null"); ct.ThrowIfCancellationRequested(); - ToolSkillSynchronizer.SkillTargetInfo[] targetArray = targets.ToArray(); - return await Task.Run(() => - { - int succeeded = 0; - foreach (ToolSkillSynchronizer.SkillTargetInfo target in targetArray) + return await RunForTargetsAsync( + targets, + (ToolSkillSynchronizer.SkillTargetInfo target, CancellationToken targetCt) => { - ct.ThrowIfCancellationRequested(); SkillTargetInstaller.InstallSpecificSkillsForTarget( projectRoot, target, Array.Empty(), new[] { skill }, groupSkillsUnderUnityCliLoop, - ct); - succeeded++; - } - - return new ToolSkillSynchronizer.SkillInstallResult(targetArray.Length, succeeded); - }, ct); + targetCt); + }, + ct); } internal static async Task RemoveSpecificSkillFilesAtProjectRoot( @@ -162,6 +150,28 @@ internal static SkillInstallState GetSkillInstallStateAtProjectRoot( Debug.Assert(!string.IsNullOrEmpty(skillName), "skillName must not be null or empty"); ct.ThrowIfCancellationRequested(); + return await RunForTargetsAsync( + targets, + (ToolSkillSynchronizer.SkillTargetInfo target, CancellationToken targetCt) => + { + string targetRoot = Path.Combine(projectRoot, target.DirName); + SkillTargetInstaller.DeleteSkillDirectoryIfExists( + targetRoot, + skillName, + groupSkillsUnderUnityCliLoop, + targetCt); + }, + ct); + } + + private static async Task RunForTargetsAsync( + IEnumerable targets, + Action targetOperation, + CancellationToken ct) + { + Debug.Assert(targets != null, "targets must not be null"); + Debug.Assert(targetOperation != null, "targetOperation must not be null"); + ToolSkillSynchronizer.SkillTargetInfo[] targetArray = targets.ToArray(); return await Task.Run(() => { @@ -169,12 +179,7 @@ internal static SkillInstallState GetSkillInstallStateAtProjectRoot( foreach (ToolSkillSynchronizer.SkillTargetInfo target in targetArray) { ct.ThrowIfCancellationRequested(); - string targetRoot = Path.Combine(projectRoot, target.DirName); - SkillTargetInstaller.DeleteSkillDirectoryIfExists( - targetRoot, - skillName, - groupSkillsUnderUnityCliLoop, - ct); + targetOperation(target, ct); succeeded++; }