Flow.Launcher/Helper/AutoStartup.cs (1)

173-200: 🛠️ Refactor suggestion

Add error handling for administrator privilege settings.

The method doesn't explicitly check if setting the RunLevel was successful, which could lead to silent failures.

 private static bool ScheduleLogonTask(bool alwaysRunAsAdministrator)
 {
     using var td = TaskService.Instance.NewTask();
     td.RegistrationInfo.Description = LogonTaskDesc;
     td.Triggers.Add(new LogonTrigger { UserId = WindowsIdentity.GetCurrent().Name, Delay = TimeSpan.FromSeconds(2) });
     td.Actions.Add(Constant.ExecutablePath);

+    bool requestedElevation = false;
     // Only if the app is running as administrator, we can set the run level to highest
     if (Win32Helper.IsAdministrator() && alwaysRunAsAdministrator)
     {
         td.Principal.RunLevel = TaskRunLevel.Highest;
+        requestedElevation = true;
     }

     td.Settings.StopIfGoingOnBatteries = false;
     td.Settings.DisallowStartIfOnBatteries = false;
     td.Settings.ExecutionTimeLimit = TimeSpan.Zero;

     try
     {
-        TaskService.Instance.RootFolder.RegisterTaskDefinition(LogonTaskName, td);
+        var registeredTask = TaskService.Instance.RootFolder.RegisterTaskDefinition(LogonTaskName, td);
+        
+        // Verify that elevation was set if requested
+        if (requestedElevation && registeredTask.Definition.Principal.RunLevel != TaskRunLevel.Highest)
+        {
+            App.API.LogWarning(ClassName, "Failed to set task to run with highest privileges");
+            // We don't fail the operation, just log a warning
+        }
+        
         return true;
     }
     catch (Exception e)
     {
         App.API.LogError(ClassName, $"Failed to schedule logon task: {e}");
         return false;
     }
 }

Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml (1)

56-66: Consider adding a warning about security implications.

Running applications with administrator privileges can have security implications. Consider adding a more detailed tooltip or warning about the potential risks of always running with elevated privileges.

Example enhancement:

<cc:Card
    Title="{DynamicResource alwaysRunAsAdministrator}"
    Icon="&#xE7EF;"
-   Sub="{DynamicResource alwaysRunAsAdministratorToolTip}">
+   Sub="{DynamicResource alwaysRunAsAdministratorToolTip}"
+   Style="{StaticResource WarningCardStyle}">
    <ui:ToggleSwitch
        IsOn="{Binding AlwaysRunAsAdministrator}"
        OffContent="{DynamicResource disable}"
        OnContent="{DynamicResource enable}" />
</cc:Card>

Note: This assumes a WarningCardStyle exists or would need to be created.

Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml.cs (3)

12-13: Consider adding XML documentation and using consistent naming conventions for static fields.

Both msgBox and _result are static fields, but they use inconsistent naming conventions. Consider prefixing both with underscore for consistency, or use a consistent naming style.

-        private static UACDialog msgBox;
+        private static UACDialog _msgBox;
         private static MessageBoxResult _result = MessageBoxResult.None;

---

68-78: Clean up dialog instance consistently in button handlers.

The Button_Click method sets the dialog instance to null, but the field is static and might be accessed from other threads. Consider applying the same pattern to the KeyEsc_OnPress method.

     private void KeyEsc_OnPress(object sender, ExecutedRoutedEventArgs e)
     {
         DialogResult = false;
         Close();
+        _result = MessageBoxResult.None;
+        _msgBox = null;
     }

     private void Button_Click(object sender, RoutedEventArgs e)
     {
         if (sender == btnYes)
             _result = MessageBoxResult.Yes;
         else if (sender == btnNo)
             _result = MessageBoxResult.No;
         else
             _result = MessageBoxResult.None;
-        msgBox.Close();
-        msgBox = null;
+        _msgBox.Close();
+        _msgBox = null;
     }

---

80-85: Use consistent naming in Button_Cancel method.

The Button_Cancel method references msgBox directly, but should use the renamed field if you adopt the suggestion above.

     private void Button_Cancel(object sender, RoutedEventArgs e)
     {
         _result = MessageBoxResult.Cancel;
-        msgBox.Close();
-        msgBox = null;
+        _msgBox.Close();
+        _msgBox = null;
     }

Flow.Launcher/Helper/AutoStartup.cs (1)

120-131: Document startup method changes for administrator mode.

The code includes a good comment explaining why registry startup doesn't support administrator mode, but could benefit from additional documentation.

 public static void ChangeToViaLogonTask(bool alwaysRunAsAdministrator)
 {
     Disable(false);
     Enable(true, alwaysRunAsAdministrator);
 }

 public static void ChangeToViaRegistry()
 {
     Disable(true);
+    // Registry startup doesn't support running as administrator because it doesn't have an option
+    // to elevate privileges like the Task Scheduler does with RunLevel.Highest
     // We do not need to use alwaysRunAsAdministrator for registry, so we just set false here
     Enable(false, false);
 }

Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (5)

4-5: Namespace collision risk between WPF and WinForms types

Adding using System.Windows; to a file that is already importing System.Windows.Forms increases the likelihood of ambiguous type resolutions (MessageBox, Screen, DragEventArgs, …).
Although you currently reference the enumerations by their full namespace (MessageBoxButton, MessageBoxResult) the next developer who tries to call MessageBox.Show (or similar) will hit a compile-time ambiguity.

Two low-effort mitigations:

-using System.Windows;
+using Wpf = System.Windows;

or import only the specific WPF types you need:

using System.Windows.MessageBoxButton;
using System.Windows.MessageBoxResult;

Either option keeps the intent clear and shields you from hidden compilation errors down the road.

---

26-27: Static cache of elevation status can become stale after restart-in-place

_isAdministrator is captured once when the SettingsPaneGeneralViewModel type is first touched.
If (in a future enhancement) you decide to elevate the process without a full application restart, this flag will remain false, giving you inconsistent behaviour.

Safer alternative: turn it into a computed property or refresh it inside CheckAdminChangeAndAskForRestart() right before the comparison.

-private static readonly bool _isAdministrator = Win32Helper.IsAdministrator();
+private static bool IsAdministrator => Win32Helper.IsAdministrator();

Then replace usages accordingly.

---

72-78: Restart prompt executes even when startup-task creation fails

CheckAdminChangeAndAskForRestart() is invoked regardless of whether AutoStartup.ChangeToViaLogonTask threw.
In the failure path we show the user two dialogs: one for the error and then another asking for a restart—yet we know the scheduled task is still out of sync.

Consider short-circuiting when the startup change fails or basing the restart prompt on the success flag returned by the helper.

---

94-113: Duplicate logic – consider extracting a helper

The UseLogonTaskForStartup setter repeats almost the same try/catch & restart-check block found in StartFlowLauncherOnSystemStartup.
Extracting this into a private method (e.g. UpdateStartupMethod(bool viaLogonTask)) would keep the two setters terse and DRY.

---

141-154: Restart helper could lose original command-line context

App.API.RestartApp(AlwaysRunAsAdministrator ? "runas" : string.Empty); forwards only the elevation flag.
If the user had started Flow Launcher with extra CLI arguments (portable mode, debug flags, etc.) they will be dropped during restart.

Recommend overloading RestartApp to accept an argument builder or capture Environment.GetCommandLineArgs() and re-use them when invoking the new process.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Flow.Launcher/MainWindow.xaml.cs:598

• [nitpick] Variable name 'text' is too generic; consider renaming to 'notifyIconText' or similar to clarify its purpose.

var text = Win32Helper.IsAdministrator() ?

Flow.Launcher/PublicAPIInstance.cs:93

• Verify that UpdateManager.RestartApp supports an overload with arguments; if not, this call will fail at runtime or cause a breaking change in the public API.

UpdateManager.RestartApp(Constant.ApplicationFileName, arguments);

Flow.Launcher/App.xaml.cs (5)

427-431: Avoid assigning an empty Verb; use null when not elevating.

Prevents potential platform quirks.

-                Verb = Win32Helper.IsAdministrator() || forceAdmin ? "runas" : ""
+                Verb = Win32Helper.IsAdministrator() || forceAdmin ? "runas" : null

---

433-434: Replace blind sleep with a process-based wait (or shorten).

More deterministic than Thread.Sleep.

-            Process.Start(startInfo);
-            Thread.Sleep(500);
+            var p = Process.Start(startInfo);
+            try { p?.WaitForInputIdle(2000); } catch { /* ignore */ }

---

427-427: Nice: executable path quoted.

This fixes issues with spaces in paths.

---

60-66: Guard against UAC-cancel to prevent crash/restart loop at startup (Win32Exception 1223).

Wrap RestartApp(true) to continue startup when elevation is declined.

 using System.Diagnostics;
+using System.ComponentModel;
 ...
-            if (_settings.AlwaysRunAsAdministrator && !Win32Helper.IsAdministrator())
-            {
-                RestartApp(true);
-                return;
-            }
+            if (_settings.AlwaysRunAsAdministrator && !Win32Helper.IsAdministrator())
+            {
+                try
+                {
+                    RestartApp(true);
+                    return;
+                }
+                catch (Win32Exception ex) when (ex.NativeErrorCode == 1223) // ERROR_CANCELLED
+                {
+                    // User cancelled UAC; continue without elevation this run.
+                }
+            }

---

279-291: Also handle UAC-cancel when prompting to restart as admin from AutoStartup().

Prevent crash if the user presses Yes but cancels UAC.

-                    {
-                        RestartApp(true);
-                    }
+                    {
+                        try
+                        {
+                            RestartApp(true);
+                        }
+                        catch (System.ComponentModel.Win32Exception ex) when (ex.NativeErrorCode == 1223)
+                        {
+                            // UAC cancelled; do nothing.
+                        }
+                    }

Flow.Launcher/PublicAPIInstance.cs (4)

613-641: De-elevation path: log on failure even without errorInfo; ensure command runner CWD is stable.

• RunAsDesktopUser may return false with empty errorInfo → add a fallback log.
• Consider using the Command exe’s directory as its working dir to avoid relative-path surprises.

-                    var result = Win32Helper.RunAsDesktopUser(
-                        Constant.CommandExecutablePath,
-                        Environment.CurrentDirectory,
+                    var result = Win32Helper.RunAsDesktopUser(
+                        Constant.CommandExecutablePath,
+                        Path.GetDirectoryName(Constant.CommandExecutablePath),
                         $"-StartProcess " +
 ...
-                    if (!string.IsNullOrEmpty(errorInfo))
+                    if (!string.IsNullOrEmpty(errorInfo))
                     {
                         LogError(ClassName, $"Failed to start process {fileName} with arguments {arguments} under {workingDirectory}: {errorInfo}");
                     }
+                    else if (!result)
+                    {
+                        LogError(ClassName, $"Failed to start process {fileName} with arguments {arguments} under {workingDirectory}: no error info.");
+                    }

---

643-651: Normalize empty verb to null in non-admin path.

Avoids passing empty string to ProcessStartInfo.Verb.

-                    Verb = verb,
+                    Verb = string.IsNullOrEmpty(verb) ? null : verb,

---

662-690: Prefer ArgumentList when not using shell execute (avoids brittle manual quoting).

Use native ArgumentList for non-admin launches; keep current behavior otherwise.

-        public bool StartProcess(string fileName, string workingDirectory = "", Collection<string> argumentList = null, bool useShellExecute = false, string verb = "", bool createNoWindow = false) =>
-            StartProcess(fileName, workingDirectory, JoinArgumentList(argumentList), useShellExecute, verb, createNoWindow);
+        public bool StartProcess(string fileName, string workingDirectory = "", Collection<string> argumentList = null, bool useShellExecute = false, string verb = "", bool createNoWindow = false)
+        {
+            // If elevated we must hop via the command runner; fall back to string-args path.
+            if (Win32Helper.IsAdministrator() || useShellExecute)
+                return StartProcess(fileName, workingDirectory, JoinArgumentList(argumentList), useShellExecute, verb, createNoWindow);
+
+            try
+            {
+                workingDirectory = string.IsNullOrEmpty(workingDirectory) ? Environment.CurrentDirectory : workingDirectory;
+                var info = new ProcessStartInfo
+                {
+                    FileName = fileName,
+                    WorkingDirectory = workingDirectory,
+                    UseShellExecute = false,
+                    Verb = string.IsNullOrEmpty(verb) ? null : verb,
+                    CreateNoWindow = createNoWindow
+                };
+                if (argumentList != null)
+                    foreach (var a in argumentList)
+                        info.ArgumentList.Add(a ?? string.Empty);
+                Process.Start(info)?.Dispose();
+                return true;
+            }
+            catch (Exception e)
+            {
+                LogException(ClassName, $"Failed to start process {fileName} with ArgumentList under {workingDirectory}", e);
+                return false;
+            }
+        }

---

665-675: Argument quoting helper is naive (embedded quotes, trailing backslashes).

Risk of mis-parsing. Consider using ArgumentList (above) or a Windows-compliant quoting routine.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3573
File: Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs:491-493
Timestamp: 2025-06-18T13:55:09.190Z
Learning: When opening Windows settings (like indexing options), de-elevation is not needed since these operations cannot bring security risks, even when Flow Launcher is running as administrator.

Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3854
File: Flow.Launcher/App.xaml.cs:246-262
Timestamp: 2025-07-21T09:19:49.684Z
Learning: In Flow Launcher's App.xaml.cs, the asynchronous plugin initialization task (containing AbstractPluginEnvironment.PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, and AutoPluginUpdates) does not require additional try-catch error handling according to maintainer Jack251970, as these operations are designed to handle exceptions internally.

Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3572
File: Flow.Launcher/App.xaml.cs:214-216
Timestamp: 2025-07-06T12:21:37.947Z
Learning: In Flow Launcher, the UpdatePluginManifestAsync method in PluginsManifest.cs already has comprehensive internal try-catch handling that logs exceptions and returns false on failure rather than throwing, making external try-catch wrappers unnecessary.

Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.

Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3672
File: Flow.Launcher/MainWindow.xaml.cs:318-318
Timestamp: 2025-06-08T14:12:12.842Z
Learning: In Flow.Launcher, the App.NotifyIcon static property is initialized in the App class before MainWindow creation, so null checks are not needed when accessing App.NotifyIcon in MainWindow lifecycle methods.