Support Always Run As Administrator #3573
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.
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>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Warning Rate limit exceeded@Jack251970 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 26 minutes and 48 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds an "Always run as administrator" option and integrates it across settings, startup scheduling, restart behavior, Win32 token helpers (de-/re-elevation), public API process start paths, plugins, and a new command-line helper used for desktop-user process launches. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant Settings
participant AutoStartup
participant Win32Helper
participant PublicAPI
participant CmdHelper as CommandUtility
User->>App: Start
App->>Settings: Load AlwaysRunAsAdministrator
App->>Win32Helper: IsAdministrator()
alt AlwaysRunAsAdministrator && not admin
App->>App: RestartApp(forceAdmin=true) (use Update.exe runas)
end
App->>AutoStartup: CheckIsEnabled(useLogonTask, AlwaysRunAsAdministrator)
AutoStartup->>Win32Helper: IsAdministrator() / Ensure task runlevel/path
User->>PublicAPI: StartProcess(file, verb)
alt Flow is elevated
PublicAPI->>Win32Helper: RunAsDesktopUser(...) -> uses CommandUtility to start on desktop session
else
PublicAPI->>CmdHelper: Start process normally (Process)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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
msgBoxand_resultare 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
msgBoxdirectly, 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.Formsincreases 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
_isAdministratoris captured once when theSettingsPaneGeneralViewModeltype 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.ChangeToViaLogonTaskthrew.
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
UseLogonTaskForStartupsetter 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
RestartAppto 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.Principalnamespace, 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 usingWindowsPrincipalandWindowsBuiltInRole. 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
AlwaysRunAsAdministratorproperty 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
AlwaysRunAsAdministratorsetting to theCheckIsEnabledmethod, 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
alwaysRunAsAdministratorparameter.
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 csLength 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
ChangeToViaLogonTaskcalls 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
AlwaysRunAsAdministratorsetting to theChangeToViaLogonTaskmethod, 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.
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.
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.
This comment has been minimized.
This comment has been minimized.
|
@Jack251970 Why are we introducing commandline support- Flow.Launcher.Command? |
If Flow already runs under admin, all shell commands fron Flow will be run under admin (e.g. Users runs one application and it will be run as admin if Flow is admin). So we should use another exe process to execute non-admin processes. |
|
Is there no other way to do this without introducing an exe? |
I cannot find any methods to do that |
|
Is it possible to use the existing exe? |
The existing exe should share the same process with the Flow. So we cannot do that. (Not sure if I have explained the reason correctly) |
|
Will JsonRPC plugins still start their own processes as admin? |
I have no idea about that. I think they will start process as admin if they use the API functions. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
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. + } + }
🧹 Nitpick comments (4)
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.
📜 Review details
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.
📒 Files selected for processing (3)
Flow.Launcher/App.xaml.cs(4 hunks)Flow.Launcher/Languages/en.xaml(2 hunks)Flow.Launcher/PublicAPIInstance.cs(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Flow.Launcher/Languages/en.xaml
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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.
📚 Learning: 2025-07-21T09:19:49.684Z
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.
Applied to files:
Flow.Launcher/App.xaml.cs
📚 Learning: 2025-07-06T12:21:37.947Z
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.
Applied to files:
Flow.Launcher/App.xaml.cs
📚 Learning: 2025-06-08T14:12:21.348Z
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.
Applied to files:
Flow.Launcher/PublicAPIInstance.cs
📚 Learning: 2025-06-08T14:12:12.842Z
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.
Applied to files:
Flow.Launcher/PublicAPIInstance.cs
🧬 Code graph analysis (1)
Flow.Launcher/PublicAPIInstance.cs (5)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (6)
RestartApp(35-35)RestartAppAsAdmin(40-40)StartProcess(651-651)StartProcess(666-666)LogError(314-314)LogException(320-320)Flow.Launcher.Core/Plugin/JsonRPCV2Models/JsonRPCPublicAPI.cs (2)
RestartApp(27-30)LogError(158-161)Flow.Launcher.Infrastructure/Win32Helper.cs (3)
Win32Helper(33-1065)IsAdministrator(913-918)RunAsDesktopUser(924-1062)Flow.Launcher.Infrastructure/Constant.cs (1)
Constant(7-60)Plugins/Flow.Launcher.Plugin.Shell/Main.cs (2)
ProcessStartInfo(190-328)Process(330-339)
🪛 GitHub Actions: Check Spelling
Flow.Launcher/App.xaml.cs
[warning] 216-216: Unrecognized spelling: 'Loadertask'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (1)
Flow.Launcher/PublicAPIInstance.cs (1)
163-164: LGTM: ShellRun now routes through StartProcess.Honors elevation/de-elevation logic centrally.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Flow.Launcher/MainWindow.xaml.cs (1)
825-825: Fix spelling to unblock Check Spelling pipelineReplace “work around” with “workaround”.
- // Initialize call twice to work around multi-display alignment issue- https://github.com/Flow-Launcher/Flow.Launcher/issues/2910 + // Initialize call twice to workaround multi-display alignment issue - https://github.com/Flow-Launcher/Flow.Launcher/issues/2910Also applies to: 849-849
♻️ Duplicate comments (5)
Plugins/Flow.Launcher.Plugin.Shell/Main.cs (1)
83-84: Verify delegate contract: returned Process is unused downstreamThese call sites now pass a delegate that always returns null. Confirm ShellCommand.Execute (and any consumers) never dereference the returned Process.
#!/usr/bin/env bash # Find ShellCommand.Execute signature and usages of its delegate return value rg -nP "ShellCommand\.Execute\s*\(" -C3 --type cs rg -nP "Func<\s*ProcessStartInfo\s*,\s*Process\s*>" --type cs -C2 # Heuristically look for '.Start(' wrappers that expect a non-null Process rg -nP "\.Execute\(\s*\w+\s*,\s*\w+\s*\)" --type cs -C2Also applies to: 123-124, 153-154, 178-179, 470-471
Flow.Launcher.Infrastructure/Win32Helper.cs (1)
1026-1048: CreateProcessWithTokenW requires a mutable command-line bufferPinned strings are immutable; lpCommandLine may be modified in place. Passing a fixed string is undefined behavior and can crash. Allocate a mutable buffer and free it.
- fixed (char* appPtr = app) - // Because argv[0] is the module name, C programmers generally repeat the module name as the first token in the command line - // So we add one more dash before the command line to make command line work correctly - fixed (char* cmdLinePtr = $"- {cmdLine}") - fixed (char* currentDirPtr = currentDir) - { - if (!PInvoke.CreateProcessWithToken( - hPrimaryToken, - // If you need to access content in HKEY_CURRENT_USER, please set loadProfile to true - loadProfile ? CREATE_PROCESS_LOGON_FLAGS.LOGON_WITH_PROFILE : 0, - appPtr, - cmdLinePtr, - // If you do not want to create a window for console app, please set createNoWindow to true - createNoWindow ? PROCESS_CREATION_FLAGS.CREATE_NO_WINDOW : 0, - null, - currentDirPtr, - &si, - &pi)) - { - errorInfo = $"CreateProcessWithTokenW failed: {Marshal.GetLastWin32Error()}"; - goto cleanup; - } - } + fixed (char* appPtr = app) + fixed (char* currentDirPtr = currentDir) + { + char* cmdLinePtr = null; + try + { + if (!string.IsNullOrEmpty(cmdLine)) + { + var mutable = $"- {cmdLine}"; + cmdLinePtr = (char*)Marshal.StringToHGlobalUni(mutable); + } + + if (!PInvoke.CreateProcessWithToken( + hPrimaryToken, + loadProfile ? CREATE_PROCESS_LOGON_FLAGS.LOGON_WITH_PROFILE : 0, + appPtr, + cmdLinePtr, + createNoWindow ? PROCESS_CREATION_FLAGS.CREATE_NO_WINDOW : 0, + null, + currentDirPtr, + &si, + &pi)) + { + errorInfo = $"CreateProcessWithTokenW failed: {Marshal.GetLastWin32Error()}"; + goto cleanup; + } + } + finally + { + if (cmdLinePtr != null) Marshal.FreeHGlobal((nint)cmdLinePtr); + } + }Flow.Launcher/MainWindow.xaml.cs (1)
724-731: Cap notify icon text to 63 chars (Windows limit) and avoid runtime exceptionsSome locales may exceed the limit. Trim defensively.
- var text = Win32Helper.IsAdministrator() ? - Constant.FlowLauncherFullName + " " + App.API.GetTranslation("admin") : - Constant.FlowLauncherFullName; + var text = Win32Helper.IsAdministrator() + ? Constant.FlowLauncherFullName + " " + App.API.GetTranslation("admin") + : Constant.FlowLauncherFullName; + if (text.Length > 63) + text = text.Substring(0, 60) + "...";Flow.Launcher/PublicAPIInstance.cs (1)
81-95: Guard UX on UAC-cancel and avoid async void (optional).
- If elevation is declined (Win32Exception NativeErrorCode 1223), the UI remains hidden. Re-show the main window on cancel.
- Consider returning Task for composability; acknowledged as non-blocking.
- private async void RestartApp(bool runAsAdmin) + private async void RestartApp(bool runAsAdmin) { _mainVM.Hide(); ... - App.RestartApp(runAsAdmin); + try + { + App.RestartApp(runAsAdmin); + } + catch (Win32Exception ex) when (ex.NativeErrorCode == 1223) + { + // UAC cancelled; restore UI. + _mainVM.Show(); + } }Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (1)
120-145: Fix: missing OnPropertyChanged and restart prompt gated too narrowly
- No OnPropertyChanged means bindings won’t update if the VM property is changed programmatically.
- Restart prompt currently runs only when both StartFlowLauncherOnSystemStartup and UseLogonTaskForStartup are true; users who toggle this while Flow is started manually won’t be prompted and will keep the wrong elevation until a manual restart.
- Error UI here uses ShowMsg while peers use ShowMsgError.
Apply this refactor to make behavior consistent and notify bindings.
public bool AlwaysRunAsAdministrator { get => Settings.AlwaysRunAsAdministrator; set { - if (AlwaysRunAsAdministrator == value) return; - - Settings.AlwaysRunAsAdministrator = value; - - if (StartFlowLauncherOnSystemStartup && UseLogonTaskForStartup) - { - try - { - AutoStartup.ChangeToViaLogonTask(value); - } - catch (Exception e) - { - App.API.ShowMsg(App.API.GetTranslation("setAutoStartFailed"), e.Message); - } - - // If we have enabled logon task startup, we need to check if we need to restart the app - // even if we encounter an error while setting the startup method - CheckAdminChangeAndAskForRestart(); - } + if (Settings.AlwaysRunAsAdministrator == value) + return; + + Settings.AlwaysRunAsAdministrator = value; + + if (StartFlowLauncherOnSystemStartup && UseLogonTaskForStartup) + { + try + { + AutoStartup.ChangeToViaLogonTask(value); + } + catch (Exception e) + { + App.API.ShowMsgError(App.API.GetTranslation("setAutoStartFailed"), e.Message); + } + } + + // Prompt regardless of how the app was started. + CheckAdminChangeAndAskForRestart(); + OnPropertyChanged(); } }
🧹 Nitpick comments (9)
Plugins/Flow.Launcher.Plugin.Shell/Main.cs (1)
20-20: Static PluginInitContext: confirm no multi-instance/reload hazardsStatic Context can be clobbered if the plugin is reloaded or instantiated in tests. If multiple instances are impossible by design, fine; otherwise consider keeping it instance-scoped.
Flow.Launcher.Infrastructure/Win32Helper.cs (1)
1050-1061: Good: handle cleanup on both success and failure pathsThe prior handle leaks are addressed; all kernel handles are closed.
Consider wrapping these in SafeHandle/using to make future changes safer.
Flow.Launcher/MainWindow.xaml.cs (1)
758-766: Also update tray tooltip text on language/hotkey changesUpdateNotifyIconText refreshes menu labels only. Include _notifyIcon.Text so the “admin” suffix localizes immediately.
Example:
private void UpdateNotifyIconText() { var menu = _contextMenu; ((MenuItem)menu.Items[0]).Header = App.API.GetTranslation("iconTrayOpen") + " (" + _settings.Hotkey + ")"; ((MenuItem)menu.Items[1]).Header = App.API.GetTranslation("GameMode"); ((MenuItem)menu.Items[2]).Header = App.API.GetTranslation("PositionReset"); ((MenuItem)menu.Items[3]).Header = App.API.GetTranslation("iconTraySettings"); ((MenuItem)menu.Items[4]).Header = App.API.GetTranslation("iconTrayExit"); var text = Win32Helper.IsAdministrator() ? Constant.FlowLauncherFullName + " " + App.API.GetTranslation("admin") : Constant.FlowLauncherFullName; if (text.Length > 63) text = text.Substring(0, 60) + "..."; _notifyIcon.Text = text; }Flow.Launcher/PublicAPIInstance.cs (3)
156-161: ShellRun delegation looks good; make intent explicit (nit).Pass useShellExecute: false explicitly to avoid ambiguity.
- StartProcess(filename, arguments: args, createNoWindow: true); + StartProcess(filename, arguments: args, useShellExecute: false, createNoWindow: true);
632-637: Log failures even when errorInfo is empty; return path is fine.If result is false but errorInfo is empty, nothing is logged. Log the failure for observability.
- if (!string.IsNullOrEmpty(errorInfo)) - { - LogError(ClassName, $"Failed to start process {fileName} with arguments {arguments} under {workingDirectory}: {errorInfo}"); - } - - return result; + if (!result) + { + var msg = string.IsNullOrEmpty(errorInfo) ? "<no error info>" : errorInfo; + LogError(ClassName, $"Failed to start process {fileName} with arguments {arguments} under {workingDirectory}: {msg}"); + } + return result;Also applies to: 652-656
610-661: Optional fallback when de-elevation fails
If Win32Helper.RunAsDesktopUser returns false, consider falling back to a direct Process.Start with the current token (optionally behind a flag) to improve resilience.Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (3)
74-79: Restart check coupling is a bit tightThe restart prompt only triggers when enabling startup via logon task here. Consider delegating to a single helper invoked after state changes (see main suggestion below) to keep behavior consistent.
111-117: Minor: unify restart promptingSame note as above—prefer centralizing the “should we restart” decision to avoid drift.
135-138: Use error variant for failure UI (consistency)Other setters use ShowMsgError for startup failures; do the same here.
- App.API.ShowMsg(App.API.GetTranslation("setAutoStartFailed"), e.Message); + App.API.ShowMsgError(App.API.GetTranslation("setAutoStartFailed"), e.Message);
📜 Review details
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.
📒 Files selected for processing (9)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs(1 hunks)Flow.Launcher.Infrastructure/Win32Helper.cs(4 hunks)Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs(4 hunks)Flow.Launcher/Flow.Launcher.csproj(1 hunks)Flow.Launcher/Languages/en.xaml(2 hunks)Flow.Launcher/MainWindow.xaml.cs(1 hunks)Flow.Launcher/PublicAPIInstance.cs(6 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs(8 hunks)Plugins/Flow.Launcher.Plugin.Shell/Main.cs(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- Flow.Launcher/Languages/en.xaml
- Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs
- Flow.Launcher/Flow.Launcher.csproj
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-06-08T14:12:21.348Z
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.
Applied to files:
Flow.Launcher/PublicAPIInstance.csFlow.Launcher/MainWindow.xaml.cs
📚 Learning: 2025-06-08T14:12:12.842Z
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.
Applied to files:
Flow.Launcher/PublicAPIInstance.csFlow.Launcher/MainWindow.xaml.cs
📚 Learning: 2025-07-01T05:46:13.251Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Applied to files:
Plugins/Flow.Launcher.Plugin.Shell/Main.cs
📚 Learning: 2025-07-21T09:19:49.684Z
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.
Applied to files:
Plugins/Flow.Launcher.Plugin.Shell/Main.cs
📚 Learning: 2025-06-18T13:55:09.190Z
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.
Applied to files:
Flow.Launcher.Infrastructure/Win32Helper.cs
🧬 Code graph analysis (5)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (4)
Flow.Launcher/PublicAPIInstance.cs (6)
System(81-95)System(163-226)ShowMsg(132-133)ShowMsg(135-138)MessageBoxResult(508-511)RestartAppAsAdmin(79-79)Flow.Launcher.Infrastructure/UserSettings/Settings.cs (1)
Settings(16-618)Flow.Launcher/Helper/AutoStartup.cs (2)
AutoStartup(12-263)ChangeToViaLogonTask(151-156)Flow.Launcher.Infrastructure/Win32Helper.cs (2)
Win32Helper(33-1065)IsAdministrator(913-918)
Flow.Launcher/PublicAPIInstance.cs (5)
Flow.Launcher/App.xaml.cs (3)
RestartApp(419-454)App(33-520)App(58-125)Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (4)
RestartApp(35-35)RestartAppAsAdmin(40-40)StartProcess(651-651)StartProcess(666-666)Flow.Launcher.Infrastructure/Win32Helper.cs (3)
Win32Helper(33-1065)IsAdministrator(913-918)RunAsDesktopUser(924-1062)Flow.Launcher.Infrastructure/Constant.cs (1)
Constant(7-60)Plugins/Flow.Launcher.Plugin.Shell/Main.cs (2)
ProcessStartInfo(190-328)Process(330-339)
Plugins/Flow.Launcher.Plugin.Shell/Main.cs (3)
Flow.Launcher/PublicAPIInstance.cs (2)
StartProcess(610-657)StartProcess(659-660)Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (2)
StartProcess(651-651)StartProcess(666-666)Plugins/Flow.Launcher.Plugin.Program/Main.cs (1)
StartProcess(445-458)
Flow.Launcher.Infrastructure/Win32Helper.cs (1)
Flow.Launcher/PublicAPIInstance.cs (2)
System(81-95)System(163-226)
Flow.Launcher/MainWindow.xaml.cs (4)
Flow.Launcher.Infrastructure/Win32Helper.cs (2)
Win32Helper(33-1065)IsAdministrator(913-918)Flow.Launcher.Infrastructure/Constant.cs (1)
Constant(7-60)Flow.Launcher/PublicAPIInstance.cs (1)
GetTranslation(253-253)Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (1)
GetTranslation(180-180)
🪛 GitHub Actions: Check Spelling
Flow.Launcher/MainWindow.xaml.cs
[error] 825-825: work around matches a line_forbidden.patterns entry: \bwork[- ]arounds?\b. (forbidden-pattern)
[warning] 928-928: XRatio is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (14)
Plugins/Flow.Launcher.Plugin.Shell/Main.cs (1)
332-337: Good: argument quoting preserved via API overloadPassing argumentList avoids manual flattening/quoting bugs flagged earlier and lets PublicAPI handle escaping consistently.
Flow.Launcher.Infrastructure/Win32Helper.cs (2)
913-918: Simple and correct admin checkUsing WindowsPrincipal against current identity is the right approach here.
693-699: Comment aligns with admin-mode policyExplicitly documenting no de-elevation for Windows Settings is helpful and consistent with prior discussions.
Flow.Launcher/PublicAPIInstance.cs (4)
4-4: Import looks correct.Needed for the new StartProcess overload using Collection.
77-80: Public restart entry points read cleanly.Nice, explicit admin vs non-admin restart methods.
446-448: Good call using UseShellExecute for non-http URIs.This routes to the shell handler and respects default apps.
640-649: Non-admin start path LGTM.Uses ProcessStartInfo and respects UseShellExecute/Verb/CreateNoWindow.
Flow.Launcher.Infrastructure/UserSettings/Settings.cs (1)
485-486: LGTM: setting persisted with sane defaultProperty addition is consistent with other flags in Settings and will serialize fine.
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (6)
4-4: LGTM: needed importSystem.Windows is required for MessageBox types.
47-49: Guard clause prevents redundant workEarly-return avoids unnecessary registry/task operations.
57-58: Correctly propagate admin run-level into schedulerPassing AlwaysRunAsAdministrator to ChangeToViaLogonTask is correct.
88-90: LGTM: guard clausePrevents unnecessary reconfiguration when value unchanged.
98-99: LGTM: scheduler updated when switching to logon taskRun-level carried through correctly.
189-189: LGTM: cache portability as readonlyImmutable cache is appropriate; changing portable mode already forces restart elsewhere.
This comment has been minimized.
This comment has been minimized.
@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, users may need to always run Flow Launcher as administrator.
We can edit
RunLevelconfiguration for logon task and then Flow Launcher can be launched as admin mode during system startup.If it is enabled, Flow will check if it is run as admin during every startup.
One more thing
We need to manually config
Programplugin &Shellplugin so that it can handle running application as non-admin/admin correctly.Additionally, many other codes related to
Process.Startare checked.Resolve #2639
Test