fix: Migration checks are faster and more reliable

Assets/Tests/Editor/SetupWizardWindowTests.cs

@@ -156,7 +156,8 @@ public void HasSkillUpdateForSetupWizard_WhenTargetHasDifferentLayoutSkills_Retu
 
         [TestCase("2.1.1", "3.0.0-beta.7", true)]
         [TestCase("1.9.0", "3.0.0", true)]
-        [TestCase("", "3.0.0-beta.7", false)]
+        [TestCase("", "3.0.0-beta.7", true)]
+        [TestCase("", "4.0.0", false)]
         [TestCase("3.0.0-beta.6", "3.0.0-beta.7", false)]
         [TestCase("3.0.0-beta.7", "4.0.0", false)]
         [TestCase("not-a-version", "3.0.0-beta.7", false)]
@@ -165,7 +166,7 @@ public void ShouldAutoScanThirdPartyToolMigration_ReturnsExpectedValue(
             string currentVersion,
             bool expected)
         {
-            // Verifies that only V2-or-older to V3 package upgrades request the migration scan.
+            // Verifies that V3 startup scans run for V2 upgrades or missing prior setup state.
             bool shouldAutoScan =
                 SetupWizardWindow.ShouldAutoScanThirdPartyToolMigration(currentVersion, lastSeenVersion);
 

Assets/Tests/Editor/ThirdPartyToolMigrationWizardWindowTests.cs

@@ -1,3 +1,6 @@
+using System.Threading;
+using System.Threading.Tasks;
+
 using NUnit.Framework;
 using UnityEditor;
 using UnityEngine;
@@ -29,6 +32,67 @@ public void ShouldStartInitialRefresh_ReturnsExpectedValue(
             Assert.That(shouldStartInitialRefresh, Is.EqualTo(expected));
         }
 
+        [TestCase(false, false, false)]
+        [TestCase(true, false, true)]
+        [TestCase(true, true, false)]
+        public void ShouldOpenWindowAfterAutoScan_ReturnsExpectedValue(
+            bool hasMigrationTargets,
+            bool isCancellationRequested,
+            bool expected)
+        {
+            // Verifies that auto-scan opens the migration window only when preflight finds work.
+            bool shouldOpenWindow = ThirdPartyToolMigrationWizardWindow.ShouldOpenWindowAfterAutoScan(
+                hasMigrationTargets,
+                isCancellationRequested);
+
+            Assert.That(shouldOpenWindow, Is.EqualTo(expected));
+        }
+
+        [Test]
+        public async Task RunAutoScanAsync_WhenTargetsExist_OpensWindowAndConsumesState()
+        {
+            // Verifies that a successful auto-scan opens the migration wizard and consumes the session flag.
+            bool openedWindow = false;
+            bool consumedSessionState = false;
+            System.Exception loggedException = null;
+
+            bool didOpenWindow = await ThirdPartyToolMigrationWizardWindow.RunAutoScanAsync(
+                _ => Task.FromResult(true),
+                _ => Task.CompletedTask,
+                () => openedWindow = true,
+                () => consumedSessionState = true,
+                ex => loggedException = ex,
+                CancellationToken.None);
+
+            Assert.That(didOpenWindow, Is.True);
+            Assert.That(openedWindow, Is.True);
+            Assert.That(consumedSessionState, Is.True);
+            Assert.That(loggedException, Is.Null);
+        }
+
+        [Test]
+        public async Task RunAutoScanAsync_WhenScanThrows_LogsExceptionAndConsumesState()
+        {
+            // Verifies that failed auto-scans cannot leak the session flag or crash through async void.
+            bool openedWindow = false;
+            bool consumedSessionState = false;
+            System.InvalidOperationException expectedException = new("scan failed");
+            System.Exception loggedException = null;
+
+            bool didOpenWindow = await ThirdPartyToolMigrationWizardWindow.RunAutoScanAsync(
+                _ => Task.FromException<bool>(expectedException),
+                _ => Task.CompletedTask,
+                () => openedWindow = true,
+                () => consumedSessionState = true,
+                ex => loggedException = ex,
+                CancellationToken.None);
+
+            Assert.That(didOpenWindow, Is.False);
+            Assert.That(openedWindow, Is.False);
+            Assert.That(consumedSessionState, Is.True);
+            Assert.That(loggedException, Is.SameAs(expectedException));
+        }
+
         [TestCase(
             1,
             "1 file needs V3 custom tool migration.\n" +

Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyUsageAnalyzer.cs

@@ -60,6 +60,11 @@ internal static MigrationAssemblyUsage FindMigrationAssemblyUsage(
             {
                 string source = File.ReadAllText(csharpFilePath);
                 sourceByCSharpFilePath.Add(csharpFilePath, source);
+                if (!ThirdPartyToolMigrationRules.ContainsMigrationCandidateText(source))
+                {
+                    continue;
+                }
+
                 string assemblyDirectory = FindNearestAssemblyDirectory(
                     csharpFilePath,
                     asmdefDirectories,
@@ -69,11 +74,6 @@ internal static MigrationAssemblyUsage FindMigrationAssemblyUsage(
                     assemblyDeclaredTypeNamesByDirectory,
                     assemblyDirectory,
                     ThirdPartyToolMigrationRules.GetDeclaredTypeNames(source));
-                if (!ThirdPartyToolMigrationRules.ContainsMigrationCandidateText(source))
-                {
-                    continue;
-                }
-
                 if (ThirdPartyToolMigrationRules.ContainsLegacyCSharpApi(source))
                 {
                     legacyAssemblyDirectories.Add(assemblyDirectory);

Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyUsageAsyncAnalyzer.cs

@@ -92,6 +92,11 @@ await CreateAssemblyReferenceDirectoriesAsync(
 
                 string source = sourceFileCache.ReadAllText(csharpFilePath);
                 await progressCounter.ReportProcessedItemAsync(ct);
+                if (!ThirdPartyToolMigrationRules.ContainsMigrationCandidateText(source))
+                {
+                    continue;
+                }
+
                 string assemblyDirectory = FindNearestAssemblyDirectory(
                     csharpFilePath,
                     asmdefDirectories,
@@ -101,11 +106,6 @@ await CreateAssemblyReferenceDirectoriesAsync(
                     assemblyDeclaredTypeNamesByDirectory,
                     assemblyDirectory,
                     ThirdPartyToolMigrationRules.GetDeclaredTypeNames(source));
-                if (!ThirdPartyToolMigrationRules.ContainsMigrationCandidateText(source))
-                {
-                    continue;
-                }
-
                 if (ThirdPartyToolMigrationRules.ContainsLegacyCSharpApi(source))
                 {
                     legacyAssemblyDirectories.Add(assemblyDirectory);

Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileService.cs

@@ -15,10 +15,13 @@ namespace io.github.hatayama.UnityCliLoop.Infrastructure
     /// </summary>
     public sealed class ThirdPartyToolMigrationFileService : IThirdPartyToolMigrationPort
     {
-        private readonly object _previewCacheLock = new();
+        private readonly object _migrationCacheLock = new();
         private bool _hasCachedPreview;
         private string _cachedPreviewProjectRoot = string.Empty;
         private ThirdPartyToolMigrationPreview _cachedPreview;
+        private bool _hasCachedPlan;
+        private string _cachedPlanProjectRoot = string.Empty;
+        private MigrationPlan _cachedPlan;
 
         public ThirdPartyToolMigrationPreview PreviewMigration(string projectRoot)
         {
@@ -35,6 +38,7 @@ public ThirdPartyToolMigrationPreview PreviewMigration(string projectRoot)
                 plan.ChangedFilePaths.Count,
                 plan.ReplacementCount,
                 plan.ChangedFilePaths.ToArray());
+            StoreCachedPlan(normalizedProjectRoot, plan);
             StoreCachedPreview(normalizedProjectRoot, preview);
             return preview;
         }
@@ -58,6 +62,7 @@ public async Task<ThirdPartyToolMigrationPreview> PreviewMigrationAsync(
                 plan.ChangedFilePaths.Count,
                 plan.ReplacementCount,
                 plan.ChangedFilePaths.ToArray());
+            StoreCachedPlan(normalizedProjectRoot, plan);
             StoreCachedPreview(normalizedProjectRoot, preview);
             return preview;
         }
@@ -85,8 +90,8 @@ public ThirdPartyToolMigrationResult ApplyMigration(string projectRoot)
             Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty");
 
             string normalizedProjectRoot = NormalizeProjectRoot(projectRoot);
+            MigrationPlan plan = GetCurrentMigrationPlan(normalizedProjectRoot);
             InvalidatePreviewCache();
-            MigrationPlan plan = ThirdPartyToolMigrationPlanBuilder.Create(normalizedProjectRoot);
             foreach (MigrationFileChange change in plan.Changes)
             {
                 ThirdPartyToolMigrationFileWriter.Write(change.FilePath, change.Content);
@@ -107,14 +112,14 @@ public async Task<ThirdPartyToolMigrationResult> ApplyMigrationAsync(
             Debug.Assert(progress != null, "progress must not be null");
 
             string normalizedProjectRoot = NormalizeProjectRoot(projectRoot);
-            InvalidatePreviewCache();
-            MigrationPlan plan = await ThirdPartyToolMigrationPlanBuilder.CreateAsync(normalizedProjectRoot, progress, ct);
+            MigrationPlan plan = await GetCurrentMigrationPlanAsync(normalizedProjectRoot, progress, ct);
             // A canceled operation must not start mutating files, but an active write batch must finish as one plan.
             if (ct.IsCancellationRequested)
             {
                 return new ThirdPartyToolMigrationResult(0, 0, Array.Empty<string>());
             }
 
+            InvalidatePreviewCache();
             for (int index = 0; index < plan.Changes.Count; index++)
             {
                 MigrationFileChange change = plan.Changes[index];
@@ -133,11 +138,75 @@ public async Task<ThirdPartyToolMigrationResult> ApplyMigrationAsync(
 
         internal void InvalidatePreviewCache()
         {
-            lock (_previewCacheLock)
+            InvalidateMigrationCaches();
+        }
+
+        private MigrationPlan GetCurrentMigrationPlan(string projectRoot)
+        {
+            Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty");
+
+            CachedMigrationPlanLookup cachedPlan = GetCurrentCachedPlan(projectRoot);
+            if (cachedPlan.Found)
+            {
+                return cachedPlan.Plan;
+            }
+
+            return ThirdPartyToolMigrationPlanBuilder.Create(projectRoot);
+        }
+
+        private async Task<MigrationPlan> GetCurrentMigrationPlanAsync(
+            string projectRoot,
+            IProgress<ThirdPartyToolMigrationProgress> progress,
+            CancellationToken ct)
+        {
+            Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty");
+            Debug.Assert(progress != null, "progress must not be null");
+
+            CachedMigrationPlanLookup cachedPlan = GetCurrentCachedPlan(projectRoot);
+            if (cachedPlan.Found)
+            {
+                return cachedPlan.Plan;
+            }
+
+            return await ThirdPartyToolMigrationPlanBuilder.CreateAsync(projectRoot, progress, ct);
+        }
+
+        private CachedMigrationPlanLookup GetCurrentCachedPlan(string projectRoot)
+        {
+            Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty");
+
+            MigrationPlan cachedPlan;
+            lock (_migrationCacheLock)
+            {
+                if (!_hasCachedPlan ||
+                    !string.Equals(_cachedPlanProjectRoot, projectRoot, StringComparison.Ordinal))
+                {
+                    return CachedMigrationPlanLookup.NotFound;
+                }
+
+                cachedPlan = _cachedPlan;
+            }
+
+            ProjectFileInventory inventory = ProjectFileInventory.Create(projectRoot);
+            if (!cachedPlan.ProjectFingerprint.Matches(inventory))
+            {
+                InvalidateMigrationCaches();
+                return CachedMigrationPlanLookup.NotFound;
+            }
+
+            return CachedMigrationPlanLookup.FoundPlan(cachedPlan);
+        }
+
+        private void InvalidateMigrationCaches()
+        {
+            lock (_migrationCacheLock)
             {
                 _hasCachedPreview = false;
                 _cachedPreviewProjectRoot = string.Empty;
                 _cachedPreview = default;
+                _hasCachedPlan = false;
+                _cachedPlanProjectRoot = string.Empty;
+                _cachedPlan = default;
             }
         }
 
@@ -158,7 +227,7 @@ private bool TryGetCachedPreview(
         {
             Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty");
 
-            lock (_previewCacheLock)
+            lock (_migrationCacheLock)
             {
                 if (_hasCachedPreview &&
                     string.Equals(_cachedPreviewProjectRoot, projectRoot, StringComparison.Ordinal))
@@ -176,14 +245,45 @@ private void StoreCachedPreview(string projectRoot, ThirdPartyToolMigrationPrevi
         {
             Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty");
 
-            lock (_previewCacheLock)
+            lock (_migrationCacheLock)
             {
                 _cachedPreviewProjectRoot = projectRoot;
                 _cachedPreview = preview;
                 _hasCachedPreview = true;
             }
         }
 
+        private void StoreCachedPlan(string projectRoot, MigrationPlan plan)
+        {
+            Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty");
+
+            lock (_migrationCacheLock)
+            {
+                _cachedPlanProjectRoot = projectRoot;
+                _cachedPlan = plan;
+                _hasCachedPlan = true;
+            }
+        }
+
+        private readonly struct CachedMigrationPlanLookup
+        {
+            public static CachedMigrationPlanLookup NotFound => new(false, default);
+
+            public static CachedMigrationPlanLookup FoundPlan(MigrationPlan plan)
+            {
+                return new CachedMigrationPlanLookup(true, plan);
+            }
+
+            private CachedMigrationPlanLookup(bool found, MigrationPlan plan)
+            {
+                Found = found;
+                Plan = plan;
+            }
+
+            public bool Found { get; }
+            public MigrationPlan Plan { get; }
+        }
+
         internal static string NormalizeProjectRoot(string projectRoot)
         {
             Debug.Assert(!string.IsNullOrEmpty(projectRoot), "projectRoot must not be null or empty");