-
Notifications
You must be signed in to change notification settings - Fork 714
Stage #9301
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
* chore: update yarn.lock baseline * chore: upgrade @nestjs and related packages from v11.1.0 to v11.1.10
* feat(window): add Plugin Marketplace window and wire it in desktop-window
* feat(ui/plugins): add Plugin Marketplace UI (list, detail, upload, filters, layout, styles)
* refactor(settings): wire plugin routes and access guard; update settings module
* refactor(desktop): menu/tray entries to open Plugin Marketplace window
* refactor(core): extend window manager interface to support plugin windows
* chore(timer): update routing/module and bootstrap for plugin window integration
* feat: export new components and modules
This commit adds exports for several new components and modules from the desktop-ui-lib. This includes components related to login, agent dashboard, integrations, language, recap, settings, and time tracker. These exports make these components and modules available for use in other parts of the application.
* feat(plugins): add plugin route and tray menu item
Adds a new route for plugins and integrates a new menu item into the tray icon's context menu. This enables users to access the plugin marketplace directly from the system tray.
The `app-routing.module.ts` now includes a 'plugins' path with a guard and lazy loading for plugin-related components and routes. The `app.module.ts` imports the `PluginsModule`.
The `tray.ts` file is updated to include a new "Plugins" menu item in the tray icon. Clicking this item will open the `PluginMarketplaceWindow` if it's not already open, or focus it if it is. This feature enhances the agent's extensibility and user experience by providing easy access to plugins.
* feat(plugins): implement plugin marketplace and routing
Integrates the plugin marketplace into the application and sets up the necessary routing for plugin management. This includes adding the `PluginsModule` to both desktop and server applications, and configuring the `plugins` route with `AuthConnectionGuard` to ensure authenticated access.
The plugin marketplace window is now created and loaded in both desktop and server entry points. This feature allows for dynamic loading and management of plugins within the application.
* fix(package): add missing newline at end of package.json
* fix(plugins): ensure plugin window is retrieved after creation
When creating a new plugin marketplace window, the code immediately tried to show it without re-retrieving the window instance from the `WindowManager`. This commit ensures that the `window` variable is updated after the new window is created, so that `window.show()` is called on the correct instance.
Additionally, removed an unnecessary `toggleDevTools()` call from the `PluginMarketplaceWindow` constructor.
Also updated the `desktop-ui-lib` to correctly bind the `routerLink` directive and added `ChangeDetectionStrategy.OnPush` to `PluginLayoutComponent` for performance.
* fix(tray): use correct ui path for plugin marketplace window
When opening the plugin marketplace window, the incorrect path was being used. This commit updates the path to `appWindow.getUiPath('plugins')` to ensure the window loads correctly.
* fix(plugin-layout): remove unnecessary change detection strategy
The `ChangeDetectionStrategy.OnPush` was implicitly handled by Angular's default behavior when not explicitly set. Removing it simplifies the component definition without altering its functionality or performance.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ 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 |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
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.
3 issues found across 74 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/desktop-ui-lib/src/lib/settings/settings.component.html">
<violation number="1" location="packages/desktop-ui-lib/src/lib/settings/settings.component.html:916">
P2: Duplicate `[disabled]` attribute on this input element. The input already has `[disabled]="appSetting?.timerStarted"` above, but this second `[disabled]="true"` will always override it, making the input permanently disabled. Remove the conditional binding if the input should always be disabled, or remove this line if the disabled state should depend on `appSetting?.timerStarted`.</violation>
</file>
<file name="packages/desktop-ui-lib/src/lib/settings/settings.component.ts">
<violation number="1" location="packages/desktop-ui-lib/src/lib/settings/settings.component.ts:583">
P1: Menu items have been removed from the non-server case: `TIMER_TRACKER.SETTINGS.ADVANCED_SETTINGS` and `TIMER_TRACKER.SETTINGS.PLUGINS` are no longer included. This appears to be a significant functional change hidden within formatting updates. If this is intentional, it should be documented; if not, these items should be restored.</violation>
</file>
<file name="apps/desktop/src/index.ts">
<violation number="1" location="apps/desktop/src/index.ts:544">
P2: The `marketplace` window is declared as a local const and not tracked at module level like other windows (`alwaysOn`, `notificationWindow`, etc.). This means it won't be properly closed in `closeAllWindows()` during app shutdown, potentially causing resource leaks. Consider declaring `let marketplace: PluginMarketplaceWindow = null;` at module level (near line 118) and adding it to the `windows` array in `closeAllWindows()`.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| nbInput | ||
| fullWidth | ||
| status="basic" | ||
| [disabled]="true" |
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.
P2: Duplicate [disabled] attribute on this input element. The input already has [disabled]="appSetting?.timerStarted" above, but this second [disabled]="true" will always override it, making the input permanently disabled. Remove the conditional binding if the input should always be disabled, or remove this line if the disabled state should depend on appSetting?.timerStarted.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/desktop-ui-lib/src/lib/settings/settings.component.html, line 916:
<comment>Duplicate `[disabled]` attribute on this input element. The input already has `[disabled]="appSetting?.timerStarted"` above, but this second `[disabled]="true"` will always override it, making the input permanently disabled. Remove the conditional binding if the input should always be disabled, or remove this line if the disabled state should depend on `appSetting?.timerStarted`.</comment>
<file context>
@@ -956,123 +847,92 @@ <h4>
+ nbInput
+ fullWidth
+ status="basic"
+ [disabled]="true"
+ />
</div>
</file context>
| const allowScreenshotCapture = auth?.allowScreenshotCapture || auth?.user?.employee?.allowScreenshotCapture; | ||
| this.menus = this.isServer | ||
| ? ['TIMER_TRACKER.SETTINGS.UPDATE', 'TIMER_TRACKER.SETTINGS.ADVANCED_SETTINGS', 'MENU.ABOUT'] | ||
| : [ |
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.
P1: Menu items have been removed from the non-server case: TIMER_TRACKER.SETTINGS.ADVANCED_SETTINGS and TIMER_TRACKER.SETTINGS.PLUGINS are no longer included. This appears to be a significant functional change hidden within formatting updates. If this is intentional, it should be documented; if not, these items should be restored.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/desktop-ui-lib/src/lib/settings/settings.component.ts, line 583:
<comment>Menu items have been removed from the non-server case: `TIMER_TRACKER.SETTINGS.ADVANCED_SETTINGS` and `TIMER_TRACKER.SETTINGS.PLUGINS` are no longer included. This appears to be a significant functional change hidden within formatting updates. If this is intentional, it should be documented; if not, these items should be restored.</comment>
<file context>
@@ -518,78 +529,70 @@ export class SettingsComponent implements OnInit, AfterViewInit, OnDestroy {
+ const allowScreenshotCapture = auth?.allowScreenshotCapture || auth?.user?.employee?.allowScreenshotCapture;
+ this.menus = this.isServer
+ ? ['TIMER_TRACKER.SETTINGS.UPDATE', 'TIMER_TRACKER.SETTINGS.ADVANCED_SETTINGS', 'MENU.ABOUT']
+ : [
+ ...(allowScreenshotCapture ? ['TIMER_TRACKER.SETTINGS.SCREEN_CAPTURE'] : []),
+ 'TIMER_TRACKER.TIMER',
</file context>
| updaterWindow = await createUpdaterWindow(updaterWindow, pathWindow.timeTrackerUi, pathWindow.preloadPath); | ||
| imageView = await createImageViewerWindow(imageView, pathWindow.timeTrackerUi, pathWindow.preloadPath); | ||
|
|
||
| const marketplace = new PluginMarketplaceWindow(pathWindow.timeTrackerUi); |
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.
P2: The marketplace window is declared as a local const and not tracked at module level like other windows (alwaysOn, notificationWindow, etc.). This means it won't be properly closed in closeAllWindows() during app shutdown, potentially causing resource leaks. Consider declaring let marketplace: PluginMarketplaceWindow = null; at module level (near line 118) and adding it to the windows array in closeAllWindows().
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/index.ts, line 544:
<comment>The `marketplace` window is declared as a local const and not tracked at module level like other windows (`alwaysOn`, `notificationWindow`, etc.). This means it won't be properly closed in `closeAllWindows()` during app shutdown, potentially causing resource leaks. Consider declaring `let marketplace: PluginMarketplaceWindow = null;` at module level (near line 118) and adding it to the `windows` array in `closeAllWindows()`.</comment>
<file context>
@@ -540,6 +541,9 @@ app.on('ready', async () => {
updaterWindow = await createUpdaterWindow(updaterWindow, pathWindow.timeTrackerUi, pathWindow.preloadPath);
imageView = await createImageViewerWindow(imageView, pathWindow.timeTrackerUi, pathWindow.preloadPath);
+ const marketplace = new PluginMarketplaceWindow(pathWindow.timeTrackerUi);
+ await marketplace.loadURL();
+
</file context>
Greptile SummaryMerges two feature branches: NestJS package updates (v11.1.0 → v11.1.10) and plugin marketplace refactoring that moves plugins from settings into a dedicated window. Major Changes:
PR Description Issue: Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Menu/Tray
participant WindowManager
participant PluginMarketplaceWindow
participant Angular Router
participant PluginComponents
User->>Menu/Tray: Click "Plugins" menu item
Menu/Tray->>WindowManager: getOne(RegisteredWindow.PLUGINS)
alt Window doesn't exist
Menu/Tray->>PluginMarketplaceWindow: new PluginMarketplaceWindow(path)
PluginMarketplaceWindow->>WindowManager: register(RegisteredWindow.PLUGINS, this)
PluginMarketplaceWindow->>PluginMarketplaceWindow: loadURL()
Menu/Tray->>WindowManager: getOne(RegisteredWindow.PLUGINS)
end
WindowManager-->>Menu/Tray: return window instance
Menu/Tray->>WindowManager: show(RegisteredWindow.PLUGINS)
WindowManager->>PluginMarketplaceWindow: show()
PluginMarketplaceWindow->>Angular Router: Navigate to /plugins
Angular Router->>Angular Router: Redirect to /plugins/installed
Angular Router->>PluginComponents: Load PluginLayoutComponent
PluginComponents-->>User: Display plugin marketplace UI
|




PR
Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.
Summary by cubic
Adds a dedicated Plugin Marketplace window and top‑level /plugins route across desktop, timer, agent, and server apps, moving plugin management out of Settings and making it accessible via tray/menu. Also upgrades NestJS and Express to the latest minor versions.
New Features
Dependencies
Written for commit 9ca1a6c. Summary will update automatically on new commits.