Address review: use prefix constants instead of magic numbers, normalize paths at source

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/AliasManager.cs

@@ -52,15 +52,6 @@ public partial class AliasManager : ObservableObject
                     WeakReferenceMessenger.Default.Send<PerformCommandMessage>(topLevelCommand.GetPerformCommandMessage());
                     return true;
                 }
-                else if (_topLevelCommandManager.HasFinishedLoadingCommands)
-                {
-                    // The command no longer exists (e.g. extension uninstalled or command renamed).
-                    // Only remove the orphaned alias once extension loading has fully settled,
-                    // otherwise valid aliases can be deleted while commands are still arriving.
-                    _settingsService.UpdateSettings(
-                        s => s with { Aliases = s.Aliases.Remove(searchText) },
-                        hotReload: false);
-                }
             }
             catch
             {
@@ -118,8 +109,6 @@ public partial class AliasManager : ObservableObject
         }
 
         var keysToRemove = new List<string>();
-        var commandsToDisassociate = new List<string>();
-
         foreach (var kv in aliases)
         {
             // Look for the old aliases for the command, and remove it
@@ -132,32 +121,26 @@ public partial class AliasManager : ObservableObject
             if (newAlias is not null && kv.Value.Alias == newAlias.Alias && kv.Value.CommandId != commandId)
             {
                 keysToRemove.Add(kv.Key);
-                commandsToDisassociate.Add(kv.Value.CommandId);
+
+                // Remove alias from other TopLevelViewModels it may be assigned to
+                var topLevelCommand = _topLevelCommandManager.LookupCommand(kv.Value.CommandId);
+                if (topLevelCommand is not null)
+                {
+                    topLevelCommand.AliasText = string.Empty;
+                }
             }
         }
 
-        // Update settings atomically before touching any ViewModels.
         _settingsService.UpdateSettings(s =>
         {
             var updatedAliases = s.Aliases.RemoveRange(keysToRemove);
 
             if (newAlias is not null)
             {
-                // Use SetItem instead of Add to be resilient to any data inconsistencies
-                // (e.g. orphaned aliases whose key matches the new alias's SearchPrefix).
-                updatedAliases = updatedAliases.SetItem(newAlias.SearchPrefix, newAlias);
+                updatedAliases = updatedAliases.Add(newAlias.SearchPrefix, newAlias);
             }
 
             return s with { Aliases = updatedAliases };
         });
-
-        // After the settings update is complete, notify conflicting ViewModels to clear
-        // their alias text. Doing this after UpdateSettings avoids reentrant calls back
-        // into UpdateAlias (and Save) via the AliasText setter.
-        foreach (var conflictingCommandId in commandsToDisassociate)
-        {
-            var topLevelCommand = _topLevelCommandManager.LookupCommand(conflictingCommandId);
-            topLevelCommand?.ClearAlias();
-        }
     }
 }

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs

@@ -47,7 +47,6 @@ public sealed partial class TopLevelCommandManager : ObservableObject,
     private readonly SupersedingAsyncGate _reloadCommandsGate;
     private CancellationTokenSource _extensionLoadCts = new();
     private CancellationToken _currentExtensionLoadCancellationToken;
-    private int _pendingBackgroundExtensionLoads;
 
     public TopLevelCommandManager(IServiceProvider serviceProvider, ICommandProviderCache commandProviderCache)
     {
@@ -69,8 +68,6 @@ public sealed partial class TopLevelCommandManager : ObservableObject,
     [ObservableProperty]
     public partial bool IsLoading { get; private set; } = true;
 
-    public bool HasFinishedLoadingCommands => !IsLoading && Volatile.Read(ref _pendingBackgroundExtensionLoads) == 0;
-
     public IEnumerable<CommandProviderWrapper> CommandProviders
     {
         get
@@ -278,7 +275,6 @@ public sealed partial class TopLevelCommandManager : ObservableObject,
         _extensionLoadCts.Dispose();
         _extensionLoadCts = new();
         _currentExtensionLoadCancellationToken = _extensionLoadCts.Token;
-        Interlocked.Exchange(ref _pendingBackgroundExtensionLoads, 0);
 
         var extensionService = _serviceProvider.GetService<IExtensionService>()!;
         await extensionService.SignalStopExtensionsAsync().ConfigureAwait(false);
@@ -365,7 +361,6 @@ public sealed partial class TopLevelCommandManager : ObservableObject,
             }
             else if (r.IsTimedOut)
             {
-                TrackBackgroundExtensionLoad(ct);
                 _ = StartExtensionWhenReadyAsync(r.Extension, r.PendingStartTask, r.Stopwatch, ct);
             }
         }
@@ -445,7 +440,6 @@ public sealed partial class TopLevelCommandManager : ObservableObject,
             // It's weird to repeat the condition here, but it allows the compiler to track nullability of other properties
             if (r.IsTimedOut)
             {
-                TrackBackgroundExtensionLoad(ct);
                 _ = AppendCommandsWhenReadyAsync(r.Wrapper, r.PendingLoadTask, r.Stopwatch, ct);
             }
         }
@@ -505,10 +499,6 @@ public sealed partial class TopLevelCommandManager : ObservableObject,
         {
             Logger.LogError($"Background start/load of extension {extension.PackageFullName} failed after {sw.ElapsedMilliseconds} ms: {ex}");
         }
-        finally
-        {
-            CompleteBackgroundExtensionLoad(ct);
-        }
     }
 
     private async Task<CommandLoadResult> TryLoadCommandsAsync(CommandProviderWrapper wrapper, CancellationToken ct)
@@ -584,26 +574,6 @@ public sealed partial class TopLevelCommandManager : ObservableObject,
         {
             Logger.LogError($"Background loading of commands and bands from {wrapper.ExtensionHost?.Extension?.PackageFullName} failed after {sw.ElapsedMilliseconds} ms: {ex}");
         }
-        finally
-        {
-            CompleteBackgroundExtensionLoad(ct);
-        }
-    }
-
-    private void TrackBackgroundExtensionLoad(CancellationToken ct)
-    {
-        if (ct == _currentExtensionLoadCancellationToken)
-        {
-            Interlocked.Increment(ref _pendingBackgroundExtensionLoads);
-        }
-    }
-
-    private void CompleteBackgroundExtensionLoad(CancellationToken ct)
-    {
-        if (ct == _currentExtensionLoadCancellationToken)
-        {
-            Interlocked.Decrement(ref _pendingBackgroundExtensionLoads);
-        }
     }
 
     private void ExtensionService_OnExtensionRemoved(IExtensionService sender, IEnumerable<IExtensionWrapper> extensions)

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelViewModel.cs

@@ -331,22 +331,6 @@ public sealed partial class TopLevelViewModel : ObservableObject, IListItem, IEx
         UpdateTags();
     }
 
-    /// <summary>
-    /// Clears the alias for this command without triggering a recursive save back to settings.
-    /// Called by <see cref="AliasManager"/> after it has already updated settings atomically,
-    /// to avoid reentrant calls into <see cref="AliasManager.UpdateAlias"/> via the <see cref="AliasText"/> setter.
-    /// </summary>
-    internal void ClearAlias()
-    {
-        if (Alias is not null)
-        {
-            Alias = null;
-            OnPropertyChanged(nameof(AliasText));
-            OnPropertyChanged(nameof(IsDirectAlias));
-            UpdateTags();
-        }
-    }
-
     private void FetchAliasFromAliasManager()
     {
         var am = _serviceProvider.GetService<AliasManager>();

src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/AliasManagerTests.cs

@@ -1,148 +0,0 @@
-// Copyright (c) Microsoft Corporation
-// The Microsoft Corporation licenses this file to you under the MIT license.
-// See the LICENSE file in the project root for more information.
-
-using System;
-using System.Collections.Immutable;
-using System.Reflection;
-using System.Threading.Tasks;
-using CommunityToolkit.Mvvm.Messaging;
-using Microsoft.CmdPal.UI.ViewModels.Services;
-using Microsoft.VisualStudio.TestTools.UnitTesting;
-using Moq;
-
-namespace Microsoft.CmdPal.UI.ViewModels.UnitTests;
-
-[TestClass]
-public class AliasManagerTests
-{
-    [TestMethod]
-    public void CheckAlias_DoesNotRemoveAliasWhileInitialLoadIsStillRunning()
-    {
-        var manager = CreateTopLevelCommandManager();
-        try
-        {
-            SetIsLoading(manager, true);
-            SetPendingBackgroundExtensionLoads(manager, 0);
-
-            var alias = new CommandAlias("test", "test.command", true);
-            var settingsService = CreateSettingsService(alias, out var state);
-            var aliasManager = new AliasManager(manager, settingsService.Object);
-
-            var handled = aliasManager.CheckAlias(alias.SearchPrefix);
-
-            Assert.IsFalse(handled);
-            Assert.IsTrue(state.Current.Aliases.ContainsKey(alias.SearchPrefix));
-            settingsService.Verify(s => s.UpdateSettings(It.IsAny<Func<SettingsModel, SettingsModel>>(), false), Times.Never);
-        }
-        finally
-        {
-            Cleanup(manager);
-        }
-    }
-
-    [TestMethod]
-    public void CheckAlias_DoesNotRemoveAliasWhileBackgroundExtensionLoadsArePending()
-    {
-        var manager = CreateTopLevelCommandManager();
-        try
-        {
-            SetIsLoading(manager, false);
-            SetPendingBackgroundExtensionLoads(manager, 1);
-
-            var alias = new CommandAlias("test", "test.command", true);
-            var settingsService = CreateSettingsService(alias, out var state);
-            var aliasManager = new AliasManager(manager, settingsService.Object);
-
-            var handled = aliasManager.CheckAlias(alias.SearchPrefix);
-
-            Assert.IsFalse(handled);
-            Assert.IsTrue(state.Current.Aliases.ContainsKey(alias.SearchPrefix));
-            settingsService.Verify(s => s.UpdateSettings(It.IsAny<Func<SettingsModel, SettingsModel>>(), false), Times.Never);
-        }
-        finally
-        {
-            Cleanup(manager);
-        }
-    }
-
-    [TestMethod]
-    public void CheckAlias_RemovesAliasAfterAllCommandsFinishLoading()
-    {
-        var manager = CreateTopLevelCommandManager();
-        try
-        {
-            SetIsLoading(manager, false);
-            SetPendingBackgroundExtensionLoads(manager, 0);
-
-            var alias = new CommandAlias("test", "test.command", true);
-            var settingsService = CreateSettingsService(alias, out var state);
-            var aliasManager = new AliasManager(manager, settingsService.Object);
-
-            var handled = aliasManager.CheckAlias(alias.SearchPrefix);
-
-            Assert.IsFalse(handled);
-            Assert.IsFalse(state.Current.Aliases.ContainsKey(alias.SearchPrefix));
-            settingsService.Verify(s => s.UpdateSettings(It.IsAny<Func<SettingsModel, SettingsModel>>(), false), Times.Once);
-        }
-        finally
-        {
-            Cleanup(manager);
-        }
-    }
-
-    private static TopLevelCommandManager CreateTopLevelCommandManager()
-    {
-        var serviceProvider = new Mock<IServiceProvider>();
-        serviceProvider.Setup(p => p.GetService(typeof(TaskScheduler))).Returns(TaskScheduler.Default);
-
-        var commandProviderCache = new Mock<ICommandProviderCache>();
-        return new TopLevelCommandManager(serviceProvider.Object, commandProviderCache.Object);
-    }
-
-    private static Mock<ISettingsService> CreateSettingsService(CommandAlias alias, out SettingsState state)
-    {
-        state = new SettingsState
-        {
-            Current = new SettingsModel
-            {
-                Aliases = ImmutableDictionary<string, CommandAlias>.Empty.Add(alias.SearchPrefix, alias),
-            },
-        };
-
-        var settingsService = new Mock<ISettingsService>();
-        var settingsState = state;
-        settingsService.SetupGet(s => s.Settings).Returns(() => settingsState.Current);
-        settingsService
-            .Setup(s => s.UpdateSettings(It.IsAny<Func<SettingsModel, SettingsModel>>(), It.IsAny<bool>()))
-            .Callback((Func<SettingsModel, SettingsModel> transform, bool _) => settingsState.Current = transform(settingsState.Current));
-
-        return settingsService;
-    }
-
-    private sealed class SettingsState
-    {
-        public SettingsModel Current { get; set; } = new();
-    }
-
-    private static void SetIsLoading(TopLevelCommandManager manager, bool isLoading)
-    {
-        var setter = typeof(TopLevelCommandManager)
-            .GetProperty(nameof(TopLevelCommandManager.IsLoading))!
-            .GetSetMethod(nonPublic: true)!;
-        setter.Invoke(manager, [isLoading]);
-    }
-
-    private static void SetPendingBackgroundExtensionLoads(TopLevelCommandManager manager, int count)
-    {
-        typeof(TopLevelCommandManager)
-            .GetField("_pendingBackgroundExtensionLoads", BindingFlags.Instance | BindingFlags.NonPublic)!
-            .SetValue(manager, count);
-    }
-
-    private static void Cleanup(TopLevelCommandManager manager)
-    {
-        WeakReferenceMessenger.Default.UnregisterAll(manager);
-        manager.Dispose();
-    }
-}

src/modules/launcher/Wox.Infrastructure/Image/ImageLoader.cs

@@ -55,13 +55,18 @@ namespace Wox.Infrastructure.Image
             return fs.Read(buffer, 0, buffer.Length) == buffer.Length && pngSignature.SequenceEqual(buffer);
         }
 
+        internal static string GetNormalizedPath(string path)
+        {
+            return PathNormalization.NormalizePath(path);
+        }
+
         public static void Initialize()
         {
             _hashGenerator = new ImageHashGenerator();
 
             foreach (var icon in new[] { Constant.ErrorIcon, Constant.LightThemedErrorIcon })
             {
-                var uri = new Uri(icon);
+                var uri = new Uri(GetNormalizedPath(icon));
 
                 try
                 {
@@ -298,7 +303,7 @@ namespace Wox.Infrastructure.Image
             BitmapImage image = new BitmapImage();
             image.BeginInit();
             image.CacheOption = BitmapCacheOption.OnLoad;
-            image.UriSource = new Uri(path);
+            image.UriSource = new Uri(GetNormalizedPath(path));
             image.EndInit();
             return image;
         }

src/modules/launcher/Wox.Plugin/Constant.cs

@@ -64,7 +64,7 @@ namespace Wox.Plugin
         public static readonly string Version = FileVersionInfo.GetVersionInfo(Assembly.Location.NonNull()).ProductVersion;
 
         public static readonly int ThumbnailSize = 64;
-        public static readonly string ErrorIcon = Path.Combine(ProgramDirectory, "Assets", "PowerLauncher", "app_error.dark.png");
-        public static readonly string LightThemedErrorIcon = Path.Combine(ProgramDirectory, "Assets", "PowerLauncher", "app_error.light.png");
+        public static readonly string ErrorIcon = PathNormalization.NormalizePath(Path.Combine(ProgramDirectory, "Assets", "PowerLauncher", "app_error.dark.png"));
+        public static readonly string LightThemedErrorIcon = PathNormalization.NormalizePath(Path.Combine(ProgramDirectory, "Assets", "PowerLauncher", "app_error.light.png"));
     }
 }

src/modules/launcher/Wox.Plugin/PathNormalization.cs

@@ -0,0 +1,29 @@
+// Copyright (c) Microsoft Corporation
+// The Microsoft Corporation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+
+namespace Wox.Plugin
+{
+    public static class PathNormalization
+    {
+        private const string UncPrefix = @"\\?\UNC\";
+        private const string ExtendedPrefix = @"\\?\";
+
+        public static string NormalizePath(string path)
+        {
+            if (path.StartsWith(UncPrefix, StringComparison.OrdinalIgnoreCase))
+            {
+                return @"\\" + path.Substring(UncPrefix.Length);
+            }
+
+            if (path.StartsWith(ExtendedPrefix, StringComparison.OrdinalIgnoreCase))
+            {
+                return path.Substring(ExtendedPrefix.Length);
+            }
+
+            return path;
+        }
+    }
+}

src/modules/launcher/Wox.Test/ImageLoaderTest.cs

@@ -0,0 +1,41 @@
+// Copyright (c) Microsoft Corporation
+// The Microsoft Corporation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Wox.Plugin;
+
+namespace Wox.Test
+{
+    [TestClass]
+    public class ImageLoaderTest
+    {
+        [DataTestMethod]
+
+        // Regular Windows paths should be returned unchanged
+        [DataRow(@"C:\path\to\file.png", @"C:\path\to\file.png")]
+        [DataRow(@"C:\Program Files\PowerToys\Assets\PowerLauncher\app_error.dark.png", @"C:\Program Files\PowerToys\Assets\PowerLauncher\app_error.dark.png")]
+
+        // UNC paths should be returned unchanged
+        [DataRow(@"\\server\share\path\file.png", @"\\server\share\path\file.png")]
+
+        // Extended-length local paths (\\?\C:\...) should have the \\?\ prefix stripped
+        [DataRow(@"\\?\C:\path\to\file.png", @"C:\path\to\file.png")]
+        [DataRow(@"\\?\C:\Program Files\PowerToys\Assets\app_error.dark.png", @"C:\Program Files\PowerToys\Assets\app_error.dark.png")]
+
+        // Extended-length UNC paths (\\?\UNC\server\...) should be converted to \\server\...
+        [DataRow(@"\\?\UNC\server\share\path\file.png", @"\\server\share\path\file.png")]
+        [DataRow(@"\\?\UNC\TH50\TH50_c\Program Files\PowerToys\Assets\PowerLauncher\app_error.dark.png", @"\\TH50\TH50_c\Program Files\PowerToys\Assets\PowerLauncher\app_error.dark.png")]
+
+        // Case-insensitive matching for the prefix
+        [DataRow(@"\\?\unc\server\share\path\file.png", @"\\server\share\path\file.png")]
+        public void GetNormalizedPath_ShouldStripExtendedLengthPrefix(string input, string expected)
+        {
+            // Act
+            string result = PathNormalization.NormalizePath(input);
+
+            // Assert
+            Assert.AreEqual(expected, result);
+        }
+    }
+}