-
-
Notifications
You must be signed in to change notification settings - Fork 445
Allow setting of minimum app version plugin requirement #3932
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🥷 Code experts: Jack251970, taooceros Jack251970 has 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: ✨ Comment |
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughThe manifest update now validates fetched plugin results, parses the current app version, filters fetched plugins by a new MinimumAppVersion field, updates UserPlugins only with compatible entries, sets lastFetchedAt when applicable, and returns a success flag. A helper performs version checks. UserPlugin gains a MinimumAppVersion property. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant PM as PluginsManifest
participant Remote as Plugin Source
participant Ver as Version Utils
User->>PM: UpdateManifestAsync()
PM->>Remote: Fetch plugins
Remote-->>PM: FetchedPlugins[]
alt No plugins fetched
PM-->>User: return false
else Plugins fetched
PM->>Ver: Parse Constant.Version -> appVersion
loop For each plugin
PM->>PM: IsMinimumAppVersionSatisfied(plugin, appVersion)?
alt Not satisfied
Note over PM: Exclude plugin (log debug)
else Satisfied
Note over PM: Keep plugin
end
end
PM->>PM: Update UserPlugins with filtered list<br/>Update lastFetchedAt
PM-->>User: return true
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (3)
Flow.Launcher.Plugin/UserPlugin.cs (1)
80-83
: MinimumAppVersion: clarify SemVer format and allow whitespace/nullsUse IsNullOrWhiteSpace semantics downstream and document expected SemVer 2.0 (e.g., 2.0.0), since non-normalized inputs like " v2.0.0 " or "2.0.0.0" may appear.
Consider updating docs to say “SemVer (e.g., 2.0.0)”. No code change required here if parsing is hardened in PluginsManifest.cs.
Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs (2)
43-45
: Guard against null/empty fetch results and log onceEarly-return is fine; add a null check and a single warning so callers get a trace when the store returns nothing.
Apply this diff:
- if (results.Count == 0) - return false; + if (results == null || results.Count == 0) + { + API.LogWarn(ClassName, "Fetched plugin manifest is empty. Skipping update."); + return false; + }
79-80
: Optional: reduce log spam when many plugins are excludedIf lots of plugins are filtered, this can flood logs. Consider aggregating a count and logging a single summary line at Info with N excluded and first few IDs at Debug.
📜 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 (2)
Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs
(3 hunks)Flow.Launcher.Plugin/UserPlugin.cs
(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 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.Core/ExternalPlugins/PluginsManifest.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:
Flow.Launcher.Core/ExternalPlugins/PluginsManifest.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:
Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs
📚 Learning: 2025-07-21T09:19:19.012Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:280-292
Timestamp: 2025-07-21T09:19:19.012Z
Learning: In Flow Launcher's PluginManager.cs, the post-initialization operations (RegisterResultsUpdatedEvent, UpdatePluginMetadataTranslation, RegisterPluginActionKeywords, DialogJump.InitializeDialogJumpPlugin, and AddPluginToLists) are designed to be exception-safe and do not require additional try-catch error handling according to the maintainer Jack251970.
Applied to files:
Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs
📚 Learning: 2025-08-13T06:12:43.382Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3897
File: Flow.Launcher/ViewModel/PluginViewModel.cs:46-51
Timestamp: 2025-08-13T06:12:43.382Z
Learning: In Flow Launcher's PluginViewModel.cs, the LoadIconAsync method does not require additional try-catch error handling according to maintainer Jack251970, as the existing error handling approach is considered sufficient for the image loading operations.
Applied to files:
Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs
🧬 Code graph analysis (1)
Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs (2)
Plugins/Flow.Launcher.Plugin.PluginsManager/Utilities.cs (1)
UserPlugin
(62-78)Flow.Launcher.Infrastructure/Constant.cs (1)
Constant
(7-59)
⏰ 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). (4)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream workflow automation
- GitHub Check: gitStream.cm
🔇 Additional comments (1)
Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs (1)
7-7
: New using is appropriateNeeded for Constant.Version; no issues.
…in_flow_version Allow setting of minimum app version plugin requirement
Allows plugin to specify in the their plugin manifest, a new optional property 'MinimumAppVersion', e.g.
"MinimumAppVersion": "2.0.0"
. When the user's is running flow v1.20.2 or below, the plugin will not show up in the Store or the PluginsManager install command results.The goal is to prevent users from downloading a plugin version that uses an API method not available in their older flow version, or trying to upgrade to the latest plugin version that is not compatible with their existing outdated flow installation.
Tested: