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 Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an AlwaysRunAsAdministrator setting and UI; implements admin detection and restart-as-admin flow; adds Win32 token helpers and native declarations to launch desktop processes from elevated Flow; makes AutoStartup logon-task scheduling admin-aware; extends Public API with StartProcess/RestartAppAsAdmin and adds a small Command helper executable. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant App
participant Settings
participant Win32 as Win32Helper
participant Upd as Update.exe
User->>App: Start
App->>Settings: Read AlwaysRunAsAdministrator
alt AlwaysRunAsAdministrator == true
App->>Win32: IsAdministrator()
alt not elevated
App->>App: RestartApp(forceAdmin: true)
App->>Upd: Start Update.exe with runas (--processStartAndWait Flow.Launcher.exe)
App-->>User: Exit
else elevated
App-->>User: Continue startup (elevated)
end
else
App-->>User: Continue startup (normal)
end
sequenceDiagram
autonumber
participant UI as Settings UI
participant VM as SettingsPaneGeneralViewModel
participant Auto as AutoStartup
participant Win32 as Win32Helper
participant Task as Task Scheduler
UI->>VM: Toggle AlwaysRunAsAdministrator
VM->>Auto: ChangeToViaLogonTask(alwaysRunAsAdministrator)
Auto->>Win32: IsAdministrator()
alt Admin user
Auto->>Task: Schedule task with RunLevel Highest if flag true
else Non-admin user
Auto->>Task: Validate path & run level
alt run level mismatch
Auto-->>VM: Throw / request manual change
else path mismatch
Auto->>Task: Reschedule task
end
end
VM->>VM: If enabling and not elevated, prompt to restart as admin
sequenceDiagram
autonumber
participant Plugin
participant API as PublicAPIInstance
participant Win32 as Win32Helper
participant Cmd as Flow.Launcher.Command.exe
participant Proc as Target Process
Plugin->>API: StartProcess(file, args, useShellExecute, verb, createNoWindow)
alt Flow is elevated
API->>Win32: RunAsDesktopUser(app: Cmd, cmdLine: -StartProcess ...)
Win32->>Proc: CreateProcessWithTokenW (desktop user)
Win32-->>API: success/failure
else not elevated
API->>Proc: Process.Start(file, args, ...)
end
API-->>Plugin: bool success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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:Details
✅ 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.
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.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
Flow.Launcher.sln (1)
292-303: Restore x64/x86 mappings for Flow.Launcher.Command
The new helper exe still maps every x64/x86 configuration back to Any CPU, so you never produce true platform-specific builds—same problem noted earlier. Please wire the mappings to their matching platforms (or drop the extra configurations entirely if the project is genuinely Any CPU).- {A9976C5C-B73A-4D29-B654-EF1C0C4C9C8C}.Debug|x64.ActiveCfg = Debug|Any CPU - {A9976C5C-B73A-4D29-B654-EF1C0C4C9C8C}.Debug|x64.Build.0 = Debug|Any CPU + {A9976C5C-B73A-4D29-B654-EF1C0C4C9C8C}.Debug|x64.ActiveCfg = Debug|x64 + {A9976C5C-B73A-4D29-B654-EF1C0C4C9C8C}.Debug|x64.Build.0 = Debug|x64 - {A9976C5C-B73A-4D29-B654-EF1C0C4C9C8C}.Debug|x86.ActiveCfg = Debug|Any CPU - {A9976C5C-B73A-4D29-B654-EF1C0C4C9C8C}.Debug|x86.Build.0 = Debug|Any CPU + {A9976C5C-B73A-4D29-B654-EF1C0C4C9C8C}.Debug|x86.ActiveCfg = Debug|x86 + {A9976C5C-B73A-4D29-B654-EF1C0C4C9C8C}.Debug|x86.Build.0 = Debug|x86 - {A9976C5C-B73A-4D29-B654-EF1C0C4C9C8C}.Release|x64.ActiveCfg = Release|Any CPU - {A9976C5C-B73A-4D29-B654-EF1C0C4C9C8C}.Release|x64.Build.0 = Release|Any CPU + {A9976C5C-B73A-4D29-B654-EF1C0C4C9C8C}.Release|x64.ActiveCfg = Release|x64 + {A9976C5C-B73A-4D29-B654-EF1C0C4C9C8C}.Release|x64.Build.0 = Release|x64 - {A9976C5C-B73A-4D29-B654-EF1C0C4C9C8C}.Release|x86.ActiveCfg = Release|Any CPU - {A9976C5C-B73A-4D29-B654-EF1C0C4C9C8C}.Release|x86.Build.0 = Release|Any CPU + {A9976C5C-B73A-4D29-B654-EF1C0C4C9C8C}.Release|x86.ActiveCfg = Release|x86 + {A9976C5C-B73A-4D29-B654-EF1C0C4C9C8C}.Release|x86.Build.0 = Release|x86
🧹 Nitpick comments (1)
Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml (1)
54-64: Consider adding explicit margin for UI consistency.The implementation correctly follows the existing patterns for SettingsCard structure, bindings, and localization. The placement after startup settings is logical.
However, most other SettingsCards in this file have explicit
Marginattributes (e.g.,Margin="0 4 0 0"for related items). Consider adding a margin to maintain consistent spacing:- <ui:SettingsCard Description="{DynamicResource alwaysRunAsAdministratorToolTip}" Header="{DynamicResource alwaysRunAsAdministrator}"> + <ui:SettingsCard Margin="0 4 0 0" Description="{DynamicResource alwaysRunAsAdministratorToolTip}" Header="{DynamicResource alwaysRunAsAdministrator}">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher.sln(4 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-10-08T15:52:58.573Z
Learnt from: taooceros
PR: Flow-Launcher/Flow.Launcher#2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Applied to files:
Flow.Launcher.sln
⏰ 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: Report (PR)
- GitHub Check: build
🔇 Additional comments (1)
Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml (1)
60-62: Verified AlwaysRunAsAdministrator binding and localization resources exist. SettingsPaneGeneralViewModel defines theAlwaysRunAsAdministratorproperty, and bothalwaysRunAsAdministratorandalwaysRunAsAdministratorToolTipkeys are present in the localization files.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml (1)
68-80: LGTM! Well-structured SettingsCard for admin mode.The new SettingsCard follows the established patterns in the file with proper icon, localization bindings, and ToggleSwitch structure. The XAML syntax is correct and consistent with other settings cards.
Optional: Consider placement relative to startup settings.
The linked issue #2639 requested the option be placed "near" the startup setting. Currently, the admin card is positioned after "hideOnStartup" (lines 54-66), which places it several cards away from the "StartFlowLauncherOnSystemStartup" expander (lines 35-52). You might consider:
- Placing it as a sub-item within the StartFlowLauncherOnSystemStartup expander (lines 44-51), or
- Positioning it immediately after the expander (after line 52) with appropriate margin
However, the current placement may be intentional to group visibility/behavior settings together, so this is purely a UX consideration rather than a functional issue.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs(3 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs
⏰ 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). (1)
- GitHub Check: build
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.
Actionable comments posted: 0
♻️ Duplicate comments (3)
Flow.Launcher/App.xaml.cs (2)
447-457: Minor: Consider using null instead of empty string for Verb.While an empty string works, using
nullmore explicitly indicates "no elevation verb" when admin or forceAdmin is false.Apply this diff if you prefer more explicit code:
- Verb = Win32Helper.IsAdministrator() || forceAdmin ? "runas" : "" + Verb = Win32Helper.IsAdministrator() || forceAdmin ? "runas" : null
62-67: Handle UAC cancellation to avoid crash and restart loop.When
RestartApp(true)is called before DI is configured, a cancelled UAC prompt (Win32Exception error 1223) will crash the app and trigger a restart loop on every subsequent launch.Wrap the restart call in a try/catch block:
// Check if the application is running as administrator if (_settings.AlwaysRunAsAdministrator && !Win32Helper.IsAdministrator()) { - RestartApp(true); - return; + try + { + RestartApp(true); + return; + } + catch (System.ComponentModel.Win32Exception ex) when (ex.NativeErrorCode == 1223) + { + // User cancelled UAC; continue startup without elevation + // Optionally log or notify the user + } }Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (1)
120-144: Missing OnPropertyChanged notification breaks UI binding.The
AlwaysRunAsAdministratorsetter at line 127 updates the underlying Settings property but doesn't callOnPropertyChanged(), preventing the UI from updating when the property changes programmatically (e.g., when loading settings or if changed by code).Add the notification call after line 143:
// 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(); } + OnPropertyChanged(); }
🧹 Nitpick comments (3)
Flow.Launcher/App.xaml.cs (1)
302-313: Consider handling UAC cancellation when restarting as admin.If the user cancels the UAC prompt,
RestartApp(true)at line 312 will throw a Win32Exception, leaving the app in an inconsistent state (startup enabled but elevation status unchanged).Wrap the restart call to handle cancellation gracefully:
if (API.ShowMsgBox( API.GetTranslation("runAsAdministratorChangeAndRestart"), API.GetTranslation("runAsAdministratorChange"), MessageBoxButton.YesNo) == MessageBoxResult.Yes) { - RestartApp(true); + try + { + RestartApp(true); + } + catch (System.ComponentModel.Win32Exception ex) when (ex.NativeErrorCode == 1223) + { + // User cancelled UAC; log or notify + API.LogInfo(ClassName, "User cancelled elevation prompt"); + } }Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (1)
147-162: De-elevation not supported; document or extend.The method only handles enabling admin mode (non-admin → admin) but not disabling it (admin → non-admin). When a user disables
AlwaysRunAsAdministratorwhile running elevated, Flow remains elevated until the next manual restart, which could persist unintended elevation.Consider prompting for restart in both directions:
private void CheckAdminChangeAndAskForRestart() { - // When we change from non-admin to admin, we need to restart the app as administrator to apply the changes - // Under non-administrator, we cannot delete or set the logon task which is run as administrator - if (AlwaysRunAsAdministrator && !Win32Helper.IsAdministrator()) + var shouldBeAdmin = AlwaysRunAsAdministrator; + var isAdmin = Win32Helper.IsAdministrator(); + if (shouldBeAdmin == isAdmin) + return; + + if (App.API.ShowMsgBox( + App.API.GetTranslation("runAsAdministratorChangeAndRestart"), + App.API.GetTranslation("runAsAdministratorChange"), + MessageBoxButton.YesNo) == MessageBoxResult.Yes) { - if (App.API.ShowMsgBox( - App.API.GetTranslation("runAsAdministratorChangeAndRestart"), - App.API.GetTranslation("runAsAdministratorChange"), - MessageBoxButton.YesNo) == MessageBoxResult.Yes) + if (shouldBeAdmin) { - // Restart the app as administrator App.API.RestartAppAsAdmin(); } - } + else + { + App.API.RestartApp(); + } + } }If de-elevation is intentionally excluded, add a comment documenting this limitation.
Flow.Launcher/PublicAPIInstance.cs (1)
638-638: Minor: workingDirectory parameter not used for helper invocation.Line 638 uses
Environment.CurrentDirectoryas the working directory for the helper process, ignoring theworkingDirectoryparameter resolved at line 631. While this may work for most cases, it creates an inconsistency between admin and non-admin execution paths.Consider using the resolved
workingDirectoryfor consistency:- var result = Win32Helper.RunAsDesktopUser( Constant.CommandExecutablePath, - Environment.CurrentDirectory, + workingDirectory, $"-StartProcess " +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs(1 hunks)Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs(3 hunks)Flow.Launcher/App.xaml.cs(4 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(7 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- Flow.Launcher.Infrastructure/UserSettings/Settings.cs
- Flow.Launcher/MainWindow.xaml.cs
- Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs
🧰 Additional context used
🧬 Code graph analysis (3)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (2)
Flow.Launcher/PublicAPIInstance.cs (7)
System(81-95)System(163-226)ShowMsg(132-133)ShowMsg(135-138)GetTranslation(253-253)MessageBoxResult(525-528)RestartAppAsAdmin(79-79)Flow.Launcher/Helper/AutoStartup.cs (2)
AutoStartup(12-263)ChangeToViaLogonTask(151-156)
Flow.Launcher/PublicAPIInstance.cs (5)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (4)
RestartApp(35-35)RestartAppAsAdmin(40-40)StartProcess(663-663)StartProcess(678-678)Flow.Launcher/App.xaml.cs (3)
RestartApp(442-477)App(34-544)App(60-131)Flow.Launcher.Infrastructure/Win32Helper.cs (3)
Win32Helper(34-1180)IsAdministrator(1025-1030)RunAsDesktopUser(1036-1177)Flow.Launcher.Infrastructure/Constant.cs (1)
Constant(7-60)Plugins/Flow.Launcher.Plugin.Shell/Main.cs (2)
ProcessStartInfo(191-329)Process(331-340)
Flow.Launcher/App.xaml.cs (4)
Flow.Launcher/PublicAPIInstance.cs (3)
System(81-95)System(163-226)RestartApp(77-77)Flow.Launcher.Infrastructure/Win32Helper.cs (2)
Win32Helper(34-1180)IsAdministrator(1025-1030)Flow.Launcher/Helper/AutoStartup.cs (2)
AutoStartup(12-263)CheckIsEnabled(20-52)Flow.Launcher.Infrastructure/Constant.cs (1)
Constant(7-60)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (10)
Flow.Launcher/App.xaml.cs (1)
3-4: LGTM!The new imports support the restart functionality introduced in this PR.
Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml (1)
68-80: LGTM!The new UI element correctly binds to the AlwaysRunAsAdministrator property and uses proper localization keys.
Flow.Launcher/Languages/en.xaml (2)
73-73: LGTM!The new localization key for displaying admin status in the UI is appropriately placed and formatted.
187-190: LGTM!The new admin-related localization keys are clear and support the administrator mode functionality.
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (2)
4-4: LGTM!The new import is required for MessageBoxButton and MessageBoxResult used in the admin restart prompt.
47-48: LGTM!The early return guards prevent redundant startup configuration updates when the property value hasn't changed.
Also applies to: 88-89
Flow.Launcher/PublicAPIInstance.cs (4)
4-4: LGTM!The new import supports the Collection parameter in StartProcess overloads.
77-95: LGTM!The restart method refactoring provides a clean public API while maintaining backward compatibility. The delegation to a private async void method is acceptable since the app will exit immediately after restarting.
679-704: LGTM with note on argument escaping.The
AddDoubleQuotesandJoinArgumentListhelpers provide basic quoting for command-line arguments. While more robust escaping (handling embedded quotes and trailing backslashes) could be considered, the current implementation is acceptable for the typical use cases in this codebase.
627-674: No action needed for RunAsDesktopUser error handling.The
RunAsDesktopUserimplementation always setserrorInfowhen returningfalse, and leaves it empty on success, so the existing log andreturn resultcorrectly handle all cases.
Added multiple namespaces to the using directives in PublicAPIInstance.cs, including System.Collections, System.Linq, System.Threading, and others. This change enables access to a broader range of functionality required by the file.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/PublicAPIInstance.cs(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Flow.Launcher/PublicAPIInstance.cs (6)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (6)
RestartApp(35-35)RestartAppAsAdmin(40-40)StartProcess(663-663)StartProcess(678-678)LogError(326-326)LogException(332-332)Flow.Launcher/App.xaml.cs (3)
RestartApp(442-477)App(34-544)App(60-131)Plugins/Flow.Launcher.Plugin.Program/Main.cs (1)
StartProcess(464-477)Flow.Launcher.Infrastructure/Win32Helper.cs (3)
Win32Helper(34-1180)IsAdministrator(1025-1030)RunAsDesktopUser(1036-1177)Flow.Launcher.Infrastructure/Constant.cs (1)
Constant(7-60)Plugins/Flow.Launcher.Plugin.Shell/Main.cs (2)
ProcessStartInfo(191-329)Process(331-340)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (5)
Flow.Launcher/PublicAPIInstance.cs (5)
78-96: LGTM! RestartApp implementation is clean.The delegation pattern is clear:
RestartApp()restarts normally,RestartAppAsAdmin()restarts elevated. The privateRestartApp(bool)correctly hides the UI, saves settings, and delegates toApp.RestartApp(runAsAdmin).
157-162: LGTM! ShellRun now routes through admin-aware StartProcess.Replacing the direct shell command execution with
StartProcessensures that when Flow is elevated, shell commands are de-elevated correctly via the helper executable.
455-467: LGTM! OpenUri now uses admin-aware StartProcess.Routing non-HTTP URIs through
StartProcesswithuseShellExecute: truemaintains the shell execution behavior while ensuring proper elevation handling when Flow is elevated.
677-678: LGTM! Clean delegation to the string-arguments overload.The
Collection<string>overload correctly delegates by joining arguments into a single string.
680-705: LGTM! Helper methods implement basic argument quoting.
AddDoubleQuotesandJoinArgumentListprovide straightforward argument quoting for the command line construction. While more robust escaping was suggested in past reviews (handling embedded quotes and trailing backslashes), the maintainer has opted to keep the current implementation.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Flow.Launcher/Languages/en.xaml (1)
187-188: Consider mentioning security implications in the tooltip.The setting and tooltip are clear, but users might benefit from understanding that running as administrator affects how launched applications inherit privileges. While the PR implements safeguards, a brief mention could help users make informed decisions.
Optional enhancement:
- <system:String x:Key="alwaysRunAsAdministratorToolTip">Run Flow Launcher as administrator on startup</system:String> + <system:String x:Key="alwaysRunAsAdministratorToolTip">Run Flow Launcher as administrator on startup. Note: This may affect how launched applications receive permissions.</system:String>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/Languages/en.xaml(2 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:252-275
Timestamp: 2025-10-16T10:48:30.573Z
Learning: In Flow Launcher's App.xaml.cs, the plugin initialization block (lines 252-275) that includes PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, _mainVM.QueryResults(), and API.SaveAppAllSettings() does not require additional Task.Run wrappers or Dispatcher.InvokeAsync calls according to maintainer Jack251970, as the threading model is already safe as designed.
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:246-262
Timestamp: 2025-07-21T09:19:49.684Z
Learning: In Flow Launcher's App.xaml.cs, the asynchronous plugin initialization task (containing AbstractPluginEnvironment.PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, and AutoPluginUpdates) does not require additional try-catch error handling according to maintainer Jack251970, as these operations are designed to handle exceptions internally.
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3573
File: Plugins/Flow.Launcher.Plugin.Shell/Main.cs:330-339
Timestamp: 2025-09-06T05:32:51.575Z
Learning: In Flow.Launcher Shell plugin's StartProcess method, the maintainer Jack251970 prefers not to propagate launch failures from Context.API.StartProcess or throw exceptions when the API call returns false. Silent failure handling is intentional for shell commands in this plugin.
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3573
File: Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs:491-493
Timestamp: 2025-06-18T13:55:09.190Z
Learning: When opening Windows settings (like indexing options), de-elevation is not needed since these operations cannot bring security risks, even when Flow Launcher is running as administrator.
Learnt from: Yusyuriv
Repo: Flow-Launcher/Flow.Launcher PR: 3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.
📚 Learning: 2025-09-05T11:56:27.267Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Flow.Launcher/HotkeyControlDialog.xaml:6-6
Timestamp: 2025-09-05T11:56:27.267Z
Learning: In Flow.Launcher's migration to iNKORE.UI.WPF.Modern UI framework, dialog resource keys like PopuBGColor, PopupButtonAreaBGColor, PopupButtonAreaBorderColor, and PopupTextColor are provided by the iNKORE.UI.WPF.Modern framework itself, not defined locally in the codebase theme files.
Applied to files:
Flow.Launcher/Languages/en.xaml
📚 Learning: 2025-07-20T07:28:28.092Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3850
File: Flow.Launcher.Core/Resource/Internationalization.cs:0-0
Timestamp: 2025-07-20T07:28:28.092Z
Learning: In Flow Launcher's Internationalization class, when the Flow Launcher language directory or default language file is missing, the only viable approach is to log an error and return early - there are no fallback mechanisms or alternative recovery strategies available due to architectural constraints.
Applied to files:
Flow.Launcher/Languages/en.xaml
📚 Learning: 2024-10-08T15:52:58.573Z
Learnt from: taooceros
Repo: Flow-Launcher/Flow.Launcher PR: 2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Applied to files:
Flow.Launcher/Languages/en.xaml
📚 Learning: 2025-09-28T03:57:43.995Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3770
File: Flow.Launcher/Helper/HotKeyMapper.cs:0-0
Timestamp: 2025-09-28T03:57:43.995Z
Learning: In Flow.Launcher/Helper/HotKeyMapper.cs, the resource descriptions for arrow keys are intentionally swapped - Up/Down keys use "HotkeyLeftRightDesc" and Left/Right keys use "HotkeyUpDownDesc". This is documented with an inline comment and is intended behavior, not a bug.
Applied to files:
Flow.Launcher/Languages/en.xaml
📚 Learning: 2025-10-16T09:29:19.653Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:519-523
Timestamp: 2025-10-16T09:29:19.653Z
Learning: In Flow Launcher's PluginManager.cs QueryDialogJumpForPluginAsync method, when a DialogJump plugin is still initializing, returning null is intentional behavior. This allows the query to fall through to the default Explorer plugin, which serves as a fallback handler. This is different from QueryForPluginAsync and QueryHomeForPluginAsync, which show "still initializing" messages because they don't have fallback mechanisms.
Applied to files:
Flow.Launcher/Languages/en.xaml
📚 Learning: 2025-06-24T19:06:48.344Z
Learnt from: Koisu-unavailable
Repo: Flow-Launcher/Flow.Launcher PR: 3770
File: Flow.Launcher/ViewModel/MainViewModel.cs:0-0
Timestamp: 2025-06-24T19:06:48.344Z
Learning: In Flow.Launcher's Explorer plugin results, the SubTitle property always contains the directory containing the file. For file results, Title contains the filename and SubTitle contains the parent directory. For directory results, SubTitle contains the directory path itself.
Applied to files:
Flow.Launcher/Languages/en.xaml
📚 Learning: 2024-11-03T07:40:11.014Z
Learnt from: Yusyuriv
Repo: Flow-Launcher/Flow.Launcher PR: 3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.
Applied to files:
Flow.Launcher/Languages/en.xaml
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (1)
Flow.Launcher/Languages/en.xaml (1)
73-73: LGTM!The admin label is clear and appropriately formatted as a suffix indicator.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Flow.Launcher/App.xaml.cs (1)
62-66:⚠️ Potential issue | 🔴 CriticalHandle UAC-cancel (
Win32Exception1223) aroundRestartApp(true)calls.Line 65 and Line 315 can throw if the user cancels elevation; in these startup paths that can terminate app startup and repeatedly re-trigger on next launch.
Proposed fix
+using System.ComponentModel; ... - if (_settings.AlwaysRunAsAdministrator && !Win32Helper.IsAdministrator()) - { - RestartApp(true); - return; - } + if (_settings.AlwaysRunAsAdministrator && !Win32Helper.IsAdministrator()) + { + try + { + RestartApp(true); + return; + } + catch (Win32Exception e) when (e.NativeErrorCode == 1223) + { + _settings.AlwaysRunAsAdministrator = false; + } + } ... - RestartApp(true); + try + { + RestartApp(true); + } + catch (Win32Exception e) when (e.NativeErrorCode == 1223) + { + _settings.AlwaysRunAsAdministrator = false; + }#!/bin/bash # Verify unguarded admin-restart call sites and lack of explicit UAC-cancel handling in App.xaml.cs rg -nP 'RestartApp\(true\)' Flow.Launcher/App.xaml.cs -C3 rg -nP 'Win32Exception|NativeErrorCode\s*==\s*1223' Flow.Launcher/App.xaml.cs -C3Also applies to: 307-316
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher/App.xaml.cs` around lines 62 - 66, Wrap the unguarded elevation restart calls so UAC-cancel (Win32Exception with NativeErrorCode == 1223) is handled: around each RestartApp(true) invocation (e.g., in the _settings.AlwaysRunAsAdministrator startup branch and the other startup path near the second call) catch System.ComponentModel.Win32Exception, detect ex.NativeErrorCode == 1223, and handle it by logging/debugging a concise message and returning/continuing startup without exiting or re-triggering elevation; for other exceptions rethrow or log as before. Ensure you reference RestartApp, AlwaysRunAsAdministrator, Win32Helper.IsAdministrator, and the Win32Exception check when making the change.Flow.Launcher.Infrastructure/Win32Helper.cs (1)
1141-1158:⚠️ Potential issue | 🔴 CriticalUse a mutable command-line buffer for
CreateProcessWithTokenW.Line 1144 pins an immutable managed string as
PWSTR; this API may write into that buffer, which is unsafe and can fail/crash.Proposed fix
- 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) + { + var cmdLineText = $"- {cmdLine}"; + var cmdLineMem = Marshal.StringToHGlobalUni(cmdLineText); + try + { + if (!PInvoke.CreateProcessWithToken( + hPrimaryToken, + loadProfile ? CREATE_PROCESS_LOGON_FLAGS.LOGON_WITH_PROFILE : 0, + appPtr, + (char*)cmdLineMem, + createNoWindow ? PROCESS_CREATION_FLAGS.CREATE_NO_WINDOW : 0, + null, + currentDirPtr, + &si, + &pi)) + { + errorInfo = $"CreateProcessWithTokenW failed: {Marshal.GetLastWin32Error()}"; + goto cleanup; + } + } + finally + { + Marshal.FreeHGlobal(cmdLineMem); + } + }According to Microsoft documentation for CreateProcessWithTokenW, is lpCommandLine an in/out mutable buffer (PWSTR) that can be modified by the API?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher.Infrastructure/Win32Helper.cs` around lines 1141 - 1158, The CreateProcessWithToken call is passing a pinned immutable string (cmdLinePtr) as PWSTR but the Win32 API (CreateProcessWithTokenW) expects a mutable in/out buffer; change the code to allocate a writable, null-terminated command-line buffer (e.g. build the "- " + cmdLine into a StringBuilder or a stackalloc char[]/byte[] buffer), copy the combined command line into that mutable buffer, pin that buffer and pass its pointer to CreateProcessWithToken (replace uses of cmdLinePtr with the writable buffer pointer) so the native call can safely modify the buffer if needed; keep the existing appPtr and currentDirPtr handling and ensure proper null-termination and pin lifetime while calling CreateProcessWithToken.
🧹 Nitpick comments (1)
Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml (1)
68-80: Consider placing this setting within the startup expander or adding conditional visibility.The "Always run as administrator" option is functionally dependent on "Start Flow Launcher on system startup" being enabled, since it modifies the logon task's RunLevel. Currently, users can enable this setting even when system startup is disabled, which may cause confusion.
Consider either:
- Moving this card inside the
ui:SettingsExpander.Itemsof the startup expander (lines 44-51), or- Adding conditional visibility/enabled state based on
StartFlowLauncherOnSystemStartupIf this is intentionally independent (e.g., the setting also affects manual restarts), please disregard.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml` around lines 68 - 80, The "Always run as administrator" SettingsCard is currently independent but should be tied to the startup setting to avoid user confusion; update the XAML so the SettingsCard containing the ToggleSwitch bound to AlwaysRunAsAdministrator is either moved inside the startup ui:SettingsExpander.Items (so it only appears when editing startup options) or add a visibility/enabled binding to StartFlowLauncherOnSystemStartup (e.g., bind SettingsCard.Visibility or ToggleSwitch.IsEnabled to StartFlowLauncherOnSystemStartup via a converter) and keep the Header/Icon the same; locate the SettingsCard and ToggleSwitch by their bindings (AlwaysRunAsAdministrator) and the startup expander by ui:SettingsExpander.Items and implement the chosen change so the admin toggle is only usable or visible when StartFlowLauncherOnSystemStartup is true.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Flow.Launcher/Helper/AutoStartup.cs`:
- Around line 73-77: CheckLogonTask currently ignores the return values of
UnscheduleLogonTask() and ScheduleLogonTask(alwaysRunAsAdministrator) and
returns true even when repair fails; update CheckLogonTask to capture and
evaluate those return values and return false if either UnscheduleLogonTask or
ScheduleLogonTask fails (i.e., if they return false) so the method only returns
true when the repair succeeded; apply the same change for the other repair
branch (the second UnscheduleLogonTask()/ScheduleLogonTask(...) occurrence) so
both paths propagate failure.
---
Duplicate comments:
In `@Flow.Launcher.Infrastructure/Win32Helper.cs`:
- Around line 1141-1158: The CreateProcessWithToken call is passing a pinned
immutable string (cmdLinePtr) as PWSTR but the Win32 API
(CreateProcessWithTokenW) expects a mutable in/out buffer; change the code to
allocate a writable, null-terminated command-line buffer (e.g. build the "- " +
cmdLine into a StringBuilder or a stackalloc char[]/byte[] buffer), copy the
combined command line into that mutable buffer, pin that buffer and pass its
pointer to CreateProcessWithToken (replace uses of cmdLinePtr with the writable
buffer pointer) so the native call can safely modify the buffer if needed; keep
the existing appPtr and currentDirPtr handling and ensure proper
null-termination and pin lifetime while calling CreateProcessWithToken.
In `@Flow.Launcher/App.xaml.cs`:
- Around line 62-66: Wrap the unguarded elevation restart calls so UAC-cancel
(Win32Exception with NativeErrorCode == 1223) is handled: around each
RestartApp(true) invocation (e.g., in the _settings.AlwaysRunAsAdministrator
startup branch and the other startup path near the second call) catch
System.ComponentModel.Win32Exception, detect ex.NativeErrorCode == 1223, and
handle it by logging/debugging a concise message and returning/continuing
startup without exiting or re-triggering elevation; for other exceptions rethrow
or log as before. Ensure you reference RestartApp, AlwaysRunAsAdministrator,
Win32Helper.IsAdministrator, and the Win32Exception check when making the
change.
---
Nitpick comments:
In `@Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml`:
- Around line 68-80: The "Always run as administrator" SettingsCard is currently
independent but should be tied to the startup setting to avoid user confusion;
update the XAML so the SettingsCard containing the ToggleSwitch bound to
AlwaysRunAsAdministrator is either moved inside the startup
ui:SettingsExpander.Items (so it only appears when editing startup options) or
add a visibility/enabled binding to StartFlowLauncherOnSystemStartup (e.g., bind
SettingsCard.Visibility or ToggleSwitch.IsEnabled to
StartFlowLauncherOnSystemStartup via a converter) and keep the Header/Icon the
same; locate the SettingsCard and ToggleSwitch by their bindings
(AlwaysRunAsAdministrator) and the startup expander by ui:SettingsExpander.Items
and implement the chosen change so the admin toggle is only usable or visible
when StartFlowLauncherOnSystemStartup is true.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
Flow.Launcher.Infrastructure/NativeMethods.txtFlow.Launcher.Infrastructure/UserSettings/Settings.csFlow.Launcher.Infrastructure/Win32Helper.csFlow.Launcher/App.xaml.csFlow.Launcher/Helper/AutoStartup.csFlow.Launcher/Languages/en.xamlFlow.Launcher/MainWindow.xaml.csFlow.Launcher/SettingPages/Views/SettingsPaneGeneral.xamlPlugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.csPlugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.csPlugins/Flow.Launcher.Plugin.Program/Programs/Win32.csPlugins/Flow.Launcher.Plugin.Sys/Main.cs
✅ Files skipped from review due to trivial changes (1)
- Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs
🚧 Files skipped from review as they are similar to previous changes (5)
- Plugins/Flow.Launcher.Plugin.Sys/Main.cs
- Flow.Launcher.Infrastructure/UserSettings/Settings.cs
- Flow.Launcher.Infrastructure/NativeMethods.txt
- Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs
- Flow.Launcher/MainWindow.xaml.cs
Changed Flow.Launcher.Command.csproj and Net9.0-SelfContained.pubxml to use .NET 9.0 instead of .NET 7.0 for building and publishing. This ensures compatibility with the latest .NET runtime.
Updated Flow.Launcher.sln for Visual Studio 18 (insiders), reordered project dependencies, and replaced duplicated project configuration entries with a single set to improve build consistency. Solution Items section adjusted with no item changes.
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