-
-
Notifications
You must be signed in to change notification settings - Fork 377
Support Always Run As Administrator #3573
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
…hen running admin mode when application is under admin mode
This comment has been minimized.
This comment has been minimized.
🥷 Code experts: onesounds Jack251970, onesounds have most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for running Flow Launcher as administrator by updating startup configurations, user dialogs, and process launching logic. Key changes include:
- Adding a UAC dialog for confirming elevation and updating related UI elements.
- Modifying process launch methods to handle elevated and non-elevated launches based on user settings.
- Enhancing auto-startup and settings logic to support an "Always Run As Administrator" option.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml.cs | New UAC dialog implementation for admin confirmation. |
Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml | UI layout for the UAC dialog. |
Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs | Updates to launch processes with optional elevation and minor task configuration. |
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs | Adjustments to launch UWP packages with elevation support. |
Plugins/Flow.Launcher.Plugin.Program/Main.cs | Added IsAdmin flag to identify current process elevation state. |
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs | Updates to auto-startup handling and admin state checks in settings. |
Other files (Languages, AutoStartup, App.xaml.cs, PublicAPIInstance.cs, Win32Helper.cs, Settings.cs) | Various supporting changes to configuration, localization strings, and helper methods for administrator mode. |
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
📝 WalkthroughWalkthroughThis change introduces an option for Flow Launcher to always run as administrator on startup. It adds a new user setting, updates the UI to expose this option, modifies startup logic to respect the setting, and implements supporting infrastructure for privilege elevation and process launching. Plugins and APIs are updated to handle process creation appropriately under elevated contexts. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant Settings
participant App
participant AutoStartup
participant Win32Helper
User->>UI: Toggle "Always Run as Administrator" in settings
UI->>Settings: Update AlwaysRunAsAdministrator property
Settings->>App: Triggers restart prompt if needed
App->>AutoStartup: Configure startup with admin flag
App->>Win32Helper: Check admin status / elevate if required
App->>App: Restart as administrator if confirmed
sequenceDiagram
participant Plugin
participant PublicAPI
participant Win32Helper
participant Flow.Launcher.Command
Plugin->>PublicAPI: StartProcess(...)
alt Running as admin
PublicAPI->>Win32Helper: RunAsDesktopUser(...)
Win32Helper->>Flow.Launcher.Command: Launch process as desktop user
else Not running as admin
PublicAPI->>System.Diagnostics.Process: Start process normally
end
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🔭 Outside diff range comments (1)
Flow.Launcher/Helper/AutoStartup.cs (1)
173-200
: 🛠️ Refactor suggestionAdd 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; } }
🧹 Nitpick comments (10)
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="" - 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 typesAdding
using System.Windows;
to a file that is already importingSystem.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 callMessageBox.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 theSettingsPaneGeneralViewModel
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 whetherAutoStartup.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 helperThe
UseLogonTaskForStartup
setter repeats almost the same try/catch & restart-check block found inStartFlowLauncherOnSystemStartup
.
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 captureEnvironment.GetCommandLineArgs()
and re-use them when invoking the new process.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs
(1 hunks)Flow.Launcher.Infrastructure/Win32Helper.cs
(2 hunks)Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs
(1 hunks)Flow.Launcher/App.xaml.cs
(1 hunks)Flow.Launcher/Helper/AutoStartup.cs
(7 hunks)Flow.Launcher/Languages/en.xaml
(2 hunks)Flow.Launcher/MainWindow.xaml.cs
(1 hunks)Flow.Launcher/PublicAPIInstance.cs
(2 hunks)Flow.Launcher/Resources/Pages/WelcomePage5.xaml.cs
(1 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs
(6 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml
(1 hunks)Plugins/Flow.Launcher.Plugin.Program/Languages/en.xaml
(1 hunks)Plugins/Flow.Launcher.Plugin.Program/Main.cs
(3 hunks)Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs
(4 hunks)Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs
(5 hunks)Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml
(1 hunks)Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml.cs
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
Flow.Launcher/Resources/Pages/WelcomePage5.xaml.cs (1)
Flow.Launcher/Helper/AutoStartup.cs (2)
AutoStartup
(12-230)ChangeToViaLogonTask
(120-124)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (2)
Flow.Launcher/PublicAPIInstance.cs (1)
RestartApp
(75-75)Flow.Launcher.Core/Plugin/JsonRPCV2Models/JsonRPCPublicAPI.cs (1)
RestartApp
(27-30)
🪛 GitHub Actions: Check Spelling
Flow.Launcher/App.xaml.cs
[warning] 63-63: WMP
is not a recognized word. (unrecognized-spelling)
[warning] 106-119: Ioc
is not a recognized word. (unrecognized-spelling)
[warning] 174-174: Ioc
is not a recognized word. (unrecognized-spelling)
[warning] 199-199: Loadertask
is not a recognized word. (unrecognized-spelling)
[warning] 225-225: VSTHRD
is not a recognized word. (unrecognized-spelling)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs
[warning] 216-216: spefic
is not a recognized word. (unrecognized-spelling)
[warning] 357-357: requerying
is not a recognized word. (unrecognized-spelling)
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs
[warning] 91-94: uap
and rescap
are not recognized words. (unrecognized-spelling)
[warning] 462-462: dlgtitle
is not a recognized word. (unrecognized-spelling)
[warning] 535-535: uap
is not a recognized word. (unrecognized-spelling)
Flow.Launcher/MainWindow.xaml.cs
[warning] 61-61: WMP
is not a recognized word. (unrecognized-spelling)
[warning] 100-100: VSTHRD
is not a recognized word. (unrecognized-spelling)
[warning] 460-475: VSTHRD
is not a recognized word. (unrecognized-spelling)
[warning] 564-566: WMP
is not a recognized word. (unrecognized-spelling)
[warning] 652-678: gamemode
and positionreset
are not recognized words. (unrecognized-spelling)
[warning] 794-795: XRatio
and YRatio
are not recognized words. (unrecognized-spelling)
[warning] 1009-1018: clocksb
and iconsb
are not recognized words. (unrecognized-spelling)
Plugins/Flow.Launcher.Plugin.Program/Main.cs
[warning] 20-20: Reloadable
is not a recognized word. (unrecognized-spelling)
[warning] 46-47: unins
and uninst
are not recognized words. (unrecognized-spelling)
[warning] 50-50: Uninstaller
is not a recognized word. (unrecognized-spelling)
[warning] 66-68: desinstalar
is not a recognized word. (unrecognized-spelling)
[warning] 78-79: Uninstaller
is not a recognized word. (unrecognized-spelling)
[warning] 135-137: Uninstallers
is not a recognized word. (unrecognized-spelling)
[warning] 142-145: uninst
, uninstaller
, and Uninstaller
are not recognized words. (unrecognized-spelling)
[warning] 145-157: Prefixs
and uninstaller
are not recognized words. (unrecognized-spelling)
[warning] 389-449: dlgtitle
is not a recognized word. (unrecognized-spelling)
Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml
[warning] 2-2: UAC
is not a recognized word. (unrecognized-spelling)
Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs
[warning] 223-224: UAC
is not a recognized word. (unrecognized-spelling)
[warning] 231-231: workaround
is not a recognized word. (unrecognized-spelling)
Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml.cs
[warning] 2-2: UAC
is not a recognized word. (unrecognized-spelling)
Flow.Launcher.Infrastructure/Win32Helper.cs
[warning] 42-42: Dwm
is not a recognized word. (unrecognized-spelling)
[warning] 54-54: DWMSBT
and SYSTEMBACKDROP
are not recognized words. (unrecognized-spelling)
[warning] 55-55: DWMSBT
and SYSTEMBACKDROP
are not recognized words. (unrecognized-spelling)
[warning] 56-56: DWMSBT
and SYSTEMBACKDROP
are not recognized words. (unrecognized-spelling)
[warning] 59-59: Dwm
and PInvoke
are not recognized words. (unrecognized-spelling)
[warning] 61-61: DWMWA
, DWMWINDOWATTRIBUTE
, and SYSTEMBACKDROP
are not recognized words. (unrecognized-spelling)
[warning] 70-70: Dwm
and PInvoke
are not recognized words. (unrecognized-spelling)
[warning] 72-72: DWMWA
and DWMWINDOWATTRIBUTE
are not recognized words. (unrecognized-spelling)
[warning] 88-90: DWMWCP
is not a recognized word. (unrecognized-spelling)
[warning] 94-94: Dwm
and PInvoke
are not recognized words. (unrecognized-spelling)
[warning] 96-96: DWMWA
and DWMWINDOWATTRIBUTE
are not recognized words. (unrecognized-spelling)
[warning] 107-107: PInvoke
is not a recognized word. (unrecognized-spelling)
[warning] 162-189: GWL
is not a recognized word. (unrecognized-spelling)
[warning] 198-210: Wnd
is not a recognized word. (unrecognized-spelling)
[warning] 239-239: Wnd
is not a recognized word. (unrecognized-spelling)
[warning] 263-277: WINTAB
, Progman
, and WORKERW
are not recognized words. (unrecognized-spelling)
[warning] 517-523: hkl
is not a recognized word. (unrecognized-spelling)
[warning] 549-549: nqo
is not a recognized word. (unrecognized-spelling)
[warning] 622-646: tsf
and Tsf
are not recognized words. (unrecognized-spelling)
[warning] 682-685: Noto
is not a recognized word. (unrecognized-spelling)
[warning] 726-751: noto
is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: gitStream workflow automation
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (25)
Flow.Launcher.Infrastructure/Win32Helper.cs (2)
8-8
: New dependency added for administrator check.Good addition of the
System.Security.Principal
namespace, which is required for the newIsAdministrator()
method implementation.
758-767
: Well-implemented administrator check method.The
IsAdministrator()
method follows the standard pattern for detecting administrator privileges in Windows applications usingWindowsPrincipal
andWindowsBuiltInRole
. This centralized implementation will help maintain consistency when checking for elevated permissions throughout the application.Flow.Launcher.Infrastructure/UserSettings/Settings.cs (1)
375-375
: New setting for administrator mode.Good implementation of the
AlwaysRunAsAdministrator
property with a sensible default value offalse
. This ensures that the application doesn't unexpectedly run with elevated permissions unless explicitly configured by the user.Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (1)
36-41
: New API method for restarting with arguments.The addition of
RestartApp(string arguments)
method to the public API will allow the application to restart with custom command-line arguments, which is essential for implementing the administrator mode toggle functionality.Plugins/Flow.Launcher.Plugin.Program/Languages/en.xaml (1)
100-102
: LGTM! User Account Control strings are well-structured.The new localization strings for the UAC dialog look good. They follow the standard Windows UAC prompt format with a title, confirmation question, and program location display.
Flow.Launcher/App.xaml.cs (1)
239-239
: LGTM! Successfully added administrator parameter to auto-startup check.The change properly passes the new
AlwaysRunAsAdministrator
setting to theCheckIsEnabled
method, ensuring that auto-startup respects the administrator mode preference.Flow.Launcher/MainWindow.xaml.cs (1)
598-605
: LGTM! Notify icon now shows administrator status correctly.Good implementation of conditional text for the notify icon tooltip. This provides clear visual feedback to users when the application is running with elevated privileges.
Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml (1)
47-66
: LGTM! Well-structured UI for administrator mode settings.The new UI elements for administrator mode are well-organized. The CardGroup approach logically groups the related startup settings together, and the toggle for "Always Run as Administrator" is properly bound to the corresponding property in the view model.
I particularly like the choice of icons - the shield icon () is perfect for representing administrator privileges, making the purpose of the setting immediately clear to users.
Plugins/Flow.Launcher.Plugin.Program/Main.cs (3)
6-6
: Appropriate addition of required namespace.Adding the System.Security.Principal namespace is necessary for the WindowsIdentity and WindowsPrincipal classes used in the new IsAdministrator method.
36-36
: Good addition of an administrator status indicator.This static boolean field provides a centralized way to determine the current administrator status within the plugin, properly initialized using the IsAdministrator method.
466-471
: Well-implemented administrator check method.The IsAdministrator method follows the standard pattern for checking Windows administrator privileges:
- Gets the current Windows identity
- Creates a principal from this identity
- Checks if the principal is in the Administrator role
The implementation is clean and correctly disposes of the WindowsIdentity object with a using statement.
Flow.Launcher/Languages/en.xaml (2)
49-49
: Good addition of admin indicator label.This localized string will be used to indicate administrator mode in the UI, supporting the new feature.
135-138
: Complete set of localization strings for administrator mode.The added strings provide comprehensive text resources for the administrator mode functionality:
- Toggle label for the setting
- Tooltip explaining the functionality
- Dialog title for mode change
- Confirmation message for restart
These strings ensure the feature is properly localized and user-friendly.
Flow.Launcher/PublicAPIInstance.cs (3)
75-75
: Good addition of parameter-less overload for backward compatibility.Adding an overload that calls the parameterized version with null maintains compatibility with existing code while extending functionality.
78-78
: Enhancement of RestartApp to support command-line arguments.The method signature change allows passing arguments to the application when restarting, which is essential for the administrator mode feature.
93-93
: Properly passing arguments to the UpdateManager.The UpdateManager.RestartApp call is now correctly passing the arguments parameter, enabling restart with custom command-line options.
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs (3)
458-458
: Good refactoring to centralize launch logic.Replacing inline launch code with a call to the dedicated Launch method improves code maintainability and consistency.
502-502
: Consistent use of centralized launch method.Using the same Launch method from the context menu handler ensures consistent behavior between different launch scenarios.
514-527
: Well-implemented launch method with proper object initialization.The refactored Launch method:
- Uses object initializer syntax for ProcessStartInfo which is cleaner
- Properly sets UseShellExecute and Verb properties
- Correctly executes the process asynchronously with Task.Run
- Uses ConfigureAwait(false) to avoid context capturing
This implementation properly handles elevation requests through the "runas" verb when needed.
Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs (2)
199-202
: Simplified code by using the new Launch method.The code now uses a centralized Launch method for starting processes, which improves code organization and reusability.
271-272
: Simplified "Run as administrator" context menu implementation.The code now uses the centralized Launch method with elevated=true parameter, improving code organization.
Flow.Launcher/Helper/AutoStartup.cs (2)
20-52
: Add administrator privilege support to startup checking.The changes correctly implement the ability to run Flow Launcher with administrator privileges at startup by adding an
alwaysRunAsAdministrator
parameter.
54-84
: Add administrator privilege support to logon task checking.The changes allow checking if the logon task is set to run with administrator privileges.
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (1)
55-56
:✅ Verification successful
Verify all call-sites migrated to new
ChangeToViaLogonTask(bool)
signatureThe extra parameter has been provided here, but older call-sites elsewhere in the codebase will silently compile if they rely on overload resolution (e.g. optional parameters) or might now fail.
Please run a sweep to ensure no stale overloads remain:
🏁 Script executed:
rg -n "ChangeToViaLogonTask(" --type cs
Length of output: 134
Let’s re-run the search using a fixed-string match to avoid regex errors:
🏁 Script executed:
#!/bin/bash rg -n -F "ChangeToViaLogonTask(" --type csLength of output: 755
All
ChangeToViaLogonTask
calls now use the new bool parameter
Verified viarg -F "ChangeToViaLogonTask(" --type cs
—no parameterless calls remain.Call-sites updated:
- Resources/Pages/WelcomePage5.xaml.cs:48
- SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs:55, 94, 127
Definition in Helper/AutoStartup.cs only exposes
ChangeToViaLogonTask(bool)
, so no stale overloads exist.Flow.Launcher/Resources/Pages/WelcomePage5.xaml.cs (1)
48-48
: Code correctly implements administrator mode setting for startupThe change properly passes the
AlwaysRunAsAdministrator
setting to theChangeToViaLogonTask
method, ensuring that Flow Launcher will respect the user's preference for running with elevated privileges on system startup. This aligns with the PR objective of supporting plugins that require administrator mode.
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for launching Flow Launcher as administrator by introducing a new AlwaysRunAsAdministrator setting and updating the process launching, auto-start, and UAC dialog functionalities accordingly. Key changes include:
- Adding a new AlwaysRunAsAdministrator property in settings and updating the settings UI.
- Modifying the logon task and process launch logic throughout the application to account for administrator mode.
- Enhancing the UAC dialog and related helper methods to support elevated launch flows.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml.cs | Added a UAC dialog for confirming elevation with asynchronous image loading. |
Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs | Updated process launching logic to conditionally show the UAC dialog and to handle elevated launch. |
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs | Modified UWP package launch flow to support administrator mode with updated auto-start handling. |
Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml | Integrated a new toggle for AlwaysRunAsAdministrator in the settings UI. |
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs | Updated logon task startup logic to include the administrator mode flag. |
Flow.Launcher/Helper/AutoStartup.cs | Adjusted logon task scheduling to incorporate the AlwaysRunAsAdministrator parameter. |
Flow.Launcher/PublicAPIInstance.cs | Provided an overload of RestartApp to accept command-line arguments for admin mode restart. |
Flow.Launcher/Infrastructure/Win32Helper.cs | Added a helper method to check administrator status. |
Flow.Launcher/Infrastructure/UserSettings/Settings.cs | Introduced the AlwaysRunAsAdministrator setting property. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds support for always running Flow Launcher as administrator by extending startup configuration, command execution, and UI.
- Introduces a UAC confirmation dialog for elevated launches when already running as admin.
- Updates Win32 and UWP launch paths to handle elevated/non-elevated scenarios, including “Always Run As Administrator” toggle.
- Extends settings UI and auto-startup helper to configure logon task run level and prompt for restart on admin-mode changes.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml(.cs) | New modal dialog for UAC confirmation when launching elevated |
Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs | Refactored launch logic, added elevated flag |
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs | Updated UWP launch with elevated support |
Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml | Added toggle for “Always run as administrator” |
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs | Handles new setting, prompts restart |
Flow.Launcher/Helper/AutoStartup.cs | Propagates new flag to logon task configuration |
Flow.Launcher/PublicAPIInstance.cs | Added overload to RestartApp with arguments |
Flow.Launcher/MainWindow.xaml.cs | Displays “(Admin)” in tray icon text when elevated |
Flow.Launcher.Infrastructure/Win32Helper.cs | Exposes IsAdministrator helper |
Flow.Launcher.Infrastructure/UserSettings/Settings.cs | New AlwaysRunAsAdministrator setting |
Comments suppressed due to low confidence (2)
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);
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for running Flow Launcher as administrator by introducing a new AlwaysRunAsAdministrator setting and updating the logon task and process launch behavior accordingly. Key changes include:
- Consolidating process launching logic with a new Launch method and updating both Win32 and UWP package modules.
- Enhancing the auto startup functionality and configuration to handle administrator mode, including changes in settings, UI, and restart behavior.
- Adding a new command-line project to support de‐elevated process launching.
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs | Refactored process launch into a new Launch method to handle admin mode. |
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs | Updated to use the new Launch method for process start with elevation. |
Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml | Added UI elements for the new AlwaysRunAsAdministrator setting. |
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs | Introduced a new property and logic to sync the AlwaysRunAsAdministrator setting with the startup task. |
Flow.Launcher/PublicAPIInstance.cs | Updated process start methods with integration of desktop-user launching on administrator mode. |
Flow.Launcher/Helper/AutoStartup.cs | Adjusted logon task scheduling to respect the new always-run-as-admin flag. |
Flow.Launcher/App.xaml.cs | Updated restart logic to support an administrator restart scenario. |
Flow.Launcher.Command/* | Added a new project to support process launching commands with proper parameter parsing. |
Comments suppressed due to low confidence (3)
Flow.Launcher/Helper/AutoStartup.cs:66
- [nitpick] Consider adding an inline comment to clarify how the run level check (and subsequent rescheduling) differentiates between administrator and non-administrator startup requirements for future maintainability.
var runLevelCorrect = CheckRunLevel(task.Definition.Principal.RunLevel, alwaysRunAsAdministrator);
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs:457
- [nitpick] The previous implementation used Task.Run().ConfigureAwait(false) for launching the process asynchronously. Please verify that invoking Launch synchronously (which internally uses Task.Run) maintains the intended asynchronous behavior without risking unobserved exceptions.
Launch(shouldRunElevated);
Flow.Launcher/PublicAPIInstance.cs:611
- [nitpick] While disposing the Process instance immediately is generally acceptable, consider verifying that disposing here does not inadvertently suppress error information or exceptions from the process start. A using pattern or explicit null check might improve resource management if further processing is added later.
Process.Start(info)?.Dispose();
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Flow.Launcher/App.xaml.cs (1)
377-378
:Thread.Sleep
blocking – already flagged previously.The synchronous
Thread.Sleep(500)
is still here; earlier reviews suggested waiting on the spawned process or using an async delay to avoid freezing the UI thread.
(Tagging as duplicate because the point has been discussed.)
🧹 Nitpick comments (3)
Flow.Launcher/App.xaml.cs (3)
350-358
: XML-doc contradicts the actual behaviour.Summary says “restart … without changing the user privileges”, yet the method elevates when
forceAdmin == true
or the current process is already elevated.
Please rename the parameter / adjust the comment to avoid confusion for future maintainers.
251-263
: Potential endless UAC prompt on every startup.When
CheckIsEnabled
throwsUnauthorizedAccessException
and the user clicks “No”, the settingStartFlowLauncherOnSystemStartup
remainstrue
.
Next launch will hit the same code path and prompt again.Consider resetting the flag (or persisting a “don’t ask again” choice) to prevent a nag loop:
if (API.ShowMsgBox(... ) == MessageBoxResult.Yes) { RestartAppAsAdministrator(); } -// else do nothing +else +{ + _settings.StartFlowLauncherOnSystemStartup = false; + API.SaveAppAllSettings(); +}
381-392
: Path construction is fragile.Building the path with
"..\\Update.exe"
relies on directory separators and..
semantics.
Safer and more portable:-FileInfo fileInfo = new FileInfo(Path.Combine(Path.GetDirectoryName(entryAssembly.Location), "..\\Update.exe")); +var updateExePath = Path.Combine( + Path.GetDirectoryName(entryAssembly.Location) ?? string.Empty, + "..", + "Update.exe"); +FileInfo fileInfo = new FileInfo(updateExePath);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/App.xaml.cs
(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Report (PR)
- GitHub Check: build
var startInfo = new ProcessStartInfo | ||
{ | ||
FileName = getUpdateExe(), | ||
Arguments = $"--processStartAndWait {Constant.ExecutablePath}", | ||
UseShellExecute = true, | ||
Verb = "runas", | ||
}; | ||
Process.Start(startInfo); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quote the executable path to survive spaces.
Constant.ExecutablePath
is concatenated directly into the argument string.
If Flow Launcher lives under "Program Files"
(or any path with spaces), update.exe
will receive a truncated argument and the restart will silently fail.
-Arguments = $"--processStartAndWait {Constant.ExecutablePath}",
+Arguments = $"--processStartAndWait \"{Constant.ExecutablePath}\"",
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
var startInfo = new ProcessStartInfo | |
{ | |
FileName = getUpdateExe(), | |
Arguments = $"--processStartAndWait {Constant.ExecutablePath}", | |
UseShellExecute = true, | |
Verb = "runas", | |
}; | |
Process.Start(startInfo); | |
var startInfo = new ProcessStartInfo | |
{ | |
FileName = getUpdateExe(), | |
- Arguments = $"--processStartAndWait {Constant.ExecutablePath}", | |
+ Arguments = $"--processStartAndWait \"{Constant.ExecutablePath}\"", | |
UseShellExecute = true, | |
Verb = "runas", | |
}; | |
Process.Start(startInfo); |
🤖 Prompt for AI Agents
In Flow.Launcher/App.xaml.cs around lines 369 to 376, the argument string passed
to ProcessStartInfo does not quote Constant.ExecutablePath, which causes issues
if the path contains spaces. Fix this by surrounding Constant.ExecutablePath
with double quotes in the Arguments string to ensure the entire path is
correctly passed as a single argument to update.exe.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. Forbidden patterns 🙅 (1)In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves. These forbidden patterns matched content: s.b. workaround(s)
If the flagged items are 🤯 false positivesIf items relate to a ...
|
Support Always Run As Administrator
Since some plugins may need administrator mode for handling sth, we need to support admin mode for Flow Launcher.
We can edit
RunLevel
configuration for logon task and then Flow Launcher can be launched as admin mode.When Flow is already run under administrator mode, its restart will still use administrator mode.
One more thing
We need to manually config
Program
plugin &Shell
plugin so that it can handle running application as non-admin/admin correctly.Resolve #2639
Test