Support Logon Task for Faster Startup Experience #3218
Conversation
This comment has been minimized.
This comment has been minimized.
|
🥷 Code experts: jjw24, Yusyuriv jjw24, Yusyuriv 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: 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. |
📝 WalkthroughWalkthroughThis pull request introduces a new feature for configuring application startup in Flow Launcher by adding support for Windows Task Scheduler logon tasks. The changes span multiple files, including settings, helper classes, language resources, and UI components. The new functionality allows users to choose between traditional registry-based startup and a logon task-based approach, potentially offering a faster startup experience. The implementation includes comprehensive error handling and multilingual support for the new startup option. Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (1)
64-85: Switching startup method is done effectively.
TogglingUseLogonTaskForStartuptriggers a quick disable-enable routine to transition smoothly between registry and task-based startup. As a good-to-have, consider refactoring into a single switch function if multiple toggles become too frequent.Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml (1)
39-44: Consider adding an icon for consistency.The new card for logon task setting is missing an icon attribute, while other cards in this file use icons. Consider adding an icon for visual consistency.
Apply this diff to add an icon:
- <cc:Card Title="{DynamicResource useLogonTaskForStartup}"> + <cc:Card Title="{DynamicResource useLogonTaskForStartup}" Icon="">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs(1 hunks)Flow.Launcher/App.xaml.cs(1 hunks)Flow.Launcher/Flow.Launcher.csproj(1 hunks)Flow.Launcher/Helper/AutoStartup.cs(3 hunks)Flow.Launcher/Languages/en.xaml(1 hunks)Flow.Launcher/Languages/zh-cn.xaml(2 hunks)Flow.Launcher/Languages/zh-tw.xaml(2 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs(2 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: gitStream.cm
🔇 Additional comments (16)
Flow.Launcher/Helper/AutoStartup.cs (8)
2-4: Imports look correct for newly introduced functionalities.
No concerns found with these added using statements.Also applies to: 8-8
15-16: Consider clarifying task naming.
The constants for task name and description look fine. However, ensure that the naming remains consistent across logs and user-facing messages so users can identify the task more easily.
22-27: Centralized check is valid but consider text clarifications.
Comment indicates checking the logon task first, which aligns with the new logic. No immediate issues are found here.
70-85: Disable logic is straightforward and consistent.
The updated signature adds flexibility for disabling either the registry entry or the scheduled task. Looks good.
91-103: Enable logic is well structured.
Similar to the disable method, enabling either logon task or registry-based startup is properly separated by parameter.
111-138: Check administrative privileges for correct run level execution.
The scheduling logic is robust. The conditional for settingRunLevel.Highestis appropriate.
140-153: Unscheduling approach is appropriate.
This routine correctly removes existing tasks by name and handles exceptions gracefully.
155-160: Role check for administrative context is correct.
Implementation usingWindowsPrincipalis standard practice. No changes necessary.Flow.Launcher/App.xaml.cs (1)
122-122: Method call respects user preference for logon task startup.
Passing_settings.UseLogonTaskForStartupintoEnable()aligns with the new approach. The existing exception handling ensures graceful fallback if enabling fails.Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (2)
45-48: Selective enable logic is correct.
The property setter checks if startup is toggled on and correctly callsAutoStartup.Enable(UseLogonTaskForStartup).
50-54: Disabling multiple startup methods ensures consistency.
CallingDisable(true)followed byDisable(false)is thorough, preventing leftover startup entries.Flow.Launcher.Infrastructure/UserSettings/Settings.cs (1)
241-241: LGTM!The new property is well-placed near other startup-related settings and follows the existing code style.
Flow.Launcher/Languages/zh-cn.xaml (1)
50-50: LGTM!The translation is accurate and appropriately placed in the startup settings section.
Flow.Launcher/Languages/zh-tw.xaml (1)
50-50: LGTM!The translation is accurate and appropriately placed in the startup settings section.
Flow.Launcher/Languages/en.xaml (1)
49-49: LGTM!The string resource is well-written, follows naming conventions, and clearly explains the benefit of using a logon task for startup.
Flow.Launcher/Flow.Launcher.csproj (1)
103-103: Verify the TaskScheduler package version.The package version is fixed at 2.11.0. Let's verify if this is the latest stable version and check for any security advisories.
✅ Verification successful
TaskScheduler package version 2.11.0 is current and secure
The project is using the latest stable version available on NuGet, and no security vulnerabilities were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for latest version and security advisories of the TaskScheduler package. # Check NuGet for latest versions curl -s https://api.nuget.org/v3/registration5-gz-semver2/taskscheduler/index.json | jq -r '.items[0].upper' # Check for security advisories gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: NUGET, package: "TaskScheduler") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }'Length of output: 517
Script:
#!/bin/bash # Description: Check latest version of TaskScheduler package using alternative NuGet API endpoint curl -s "https://api.nuget.org/v3-flatcontainer/taskscheduler/index.json" | jq -r '.versions[]' | sort -V | tail -n 1Length of output: 124
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
Flow.Launcher/Helper/AutoStartup.cs (1)
44-69:⚠️ Potential issueLogic error: returns true even when the task doesn't exist.
If
taskis null, the method proceeds to returntrue, which incorrectly signals that the logon task is enabled.Apply a fix to return
falsewhen the task is not found:private static bool CheckLogonTask() { using var taskService = new TaskService(); var task = taskService.RootFolder.AllTasks.FirstOrDefault(t => t.Name == LogonTaskName); - if (task != null) + if (task == null) + { + return false; + } { try {
🧹 Nitpick comments (3)
Flow.Launcher/Helper/AutoStartup.cs (3)
22-27: Add error handling for logon task check.The logon task check should be wrapped in a try-catch block similar to the registry check to ensure robustness.
// Check if logon task is enabled -if (CheckLogonTask()) +try { - return true; + if (CheckLogonTask()) + { + return true; + } +} +catch (Exception e) +{ + Log.Error("AutoStartup", $"Ignoring non-critical logon task error (querying if enabled): {e}"); }
Line range hint
71-111: Add parameter validation.The methods should validate the state before making changes to avoid potential race conditions.
public static void Enable(bool logonTask) { + // Avoid enabling if already enabled with the same method + if (IsEnabled && CheckLogonTask() == logonTask) + { + return; + } + try {
113-161: Enhance security settings for the scheduled task.Consider adding these security-related improvements:
- Set
AllowHardTerminateto false to prevent force-stopping- Enable
RunOnlyIfNetworkAvailableif network connectivity is required- Add
SecurityDescriptorto restrict task accesstd.Settings.StopIfGoingOnBatteries = false; td.Settings.DisallowStartIfOnBatteries = false; td.Settings.ExecutionTimeLimit = TimeSpan.Zero; +td.Settings.AllowHardTerminate = false; +td.Settings.RunOnlyIfNetworkAvailable = true; + +// Restrict access to SYSTEM and Administrators +td.SecurityDescriptor = "D:(A;;FRFX;;;SY)(A;;FRFX;;;BA)";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/Helper/AutoStartup.cs(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (1)
Flow.Launcher/Helper/AutoStartup.cs (1)
8-8: LGTM!The new imports and constants are well-defined and appropriate for the task scheduler functionality.
Also applies to: 15-16
Add support for FL to use logon task instead of startup entry for faster strartup experience.
Test
After using logon task for startup, FL launches as fast as PowerToys (the center app in the tray) which uses logon task for startup as well.
Also, it launchers faster than other apps using startup entry, like Microsoft Security Center (You cannot see it here because it has not launched yet.)
Related issue: #3212.