feat(desktop): replace auto-update dialog with in-app toast notification#508
feat(desktop): replace auto-update dialog with in-app toast notification#508
Conversation
Replace the intrusive Electron dialog with a subtle toast notification in the bottom-right corner when updates are downloaded. The toast shows version info with "See changes" (opens release notes) and "Restart" buttons. - Add tRPC router for auto-update events and install action - Create UpdateToast component with dismiss button - Use EventEmitter to broadcast update events to renderer - Remove unused setMainWindow function 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds an event-driven auto-update system: main process emits update status events; a new TRPC autoUpdate router streams and controls updates; renderer subscribes to show persistent UpdateToast UI; setMainWindow was removed. (50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Main as Main Process (auto-updater)
participant Emitter as autoUpdateEmitter
participant Router as TRPC autoUpdate Router
participant Renderer as Renderer (useUpdateListener)
participant Toast as UpdateToast UI
rect rgba(238,246,255,0.8)
Note over Main,Emitter: Main emits status events
Main->>Emitter: emit(status: CHECKING/DOWNLOADING/READY/ERROR)
end
rect rgba(242,255,240,0.8)
Emitter-->>Router: observable emits event
Router-->>Renderer: tRPC subscription pushes event
Renderer->>Toast: render or dismiss toast per status
end
alt User clicks "Install" when READY
Toast->>Router: call install mutation
Router->>Main: invoke installUpdate()
end
rect rgba(255,245,240,0.8)
Note over Main,Renderer: Dev triggers (menu) call simulateDownloading/simulateReady/simulateError
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
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: 4
🧹 Nitpick comments (1)
apps/desktop/src/main/lib/auto-updater.ts (1)
109-112: Release URL construction assumes version format with 'v' prefix.The release URL is constructed as
`${RELEASES_URL}/tag/v${info.version}`(line 111), which assumes the GitHub release tag follows the formatv{version}(e.g.,v1.2.3). While this is a common convention, the version from electron-updater might not always include or need the 'v' prefix.Consider verifying the version format or making the URL construction more flexible:
const event: UpdateDownloadedEvent = { version: info.version, - releaseUrl: `${RELEASES_URL}/tag/v${info.version}`, + releaseUrl: `${RELEASES_URL}/tag/${info.version.startsWith('v') ? '' : 'v'}${info.version}`, };Or document the assumption:
+ // Note: Assumes GitHub release tags follow the format v{version} const event: UpdateDownloadedEvent = { version: info.version, releaseUrl: `${RELEASES_URL}/tag/v${info.version}`, };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/desktop/src/lib/trpc/routers/auto-update/index.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/main/lib/auto-updater.tsapps/desktop/src/main/windows/main.tsapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/renderer/components/UpdateToast/index.tsapps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsxapps/desktop/src/renderer/screens/main/index.tsx
💤 Files with no reviewable changes (1)
- apps/desktop/src/main/windows/main.ts
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/renderer/components/UpdateToast/index.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/auto-update/index.tsapps/desktop/src/main/lib/auto-updater.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/renderer/components/UpdateToast/index.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/auto-update/index.tsapps/desktop/src/main/lib/auto-updater.ts
**/{components,features}/**/[!.]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Files:
apps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsxapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsxapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/renderer/components/UpdateToast/index.ts
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/renderer/components/UpdateToast/index.ts
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/renderer/components/UpdateToast/index.tsapps/desktop/src/main/lib/auto-updater.ts
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx
**/{components,features}/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/{components,features}/**/*.tsx: Nest components in parent's components/ folder if used only once, promote to highest shared parent's components/ if used 2+ times
Use one component per file - do not combine multiple components in a single file
Files:
apps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsxapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/renderer/components/UpdateToast/index.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/auto-update/index.tsapps/desktop/src/main/lib/auto-updater.ts
apps/desktop/src/main/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Accept object parameters in IPC handlers - do not use positional parameters in ipcMain.handle()
Files:
apps/desktop/src/main/lib/auto-updater.ts
🧬 Code graph analysis (4)
apps/desktop/src/renderer/screens/main/index.tsx (1)
apps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsx (1)
useUpdateListener(10-45)
apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx (4)
apps/desktop/src/renderer/components/UpdateToast/index.ts (1)
UpdateToast(1-1)apps/desktop/src/main/lib/auto-updater.ts (1)
installUpdate(21-23)packages/ui/src/components/ui/sonner.tsx (1)
toast(40-40)packages/ui/src/components/ui/button.tsx (1)
Button(60-60)
apps/desktop/src/lib/trpc/routers/index.ts (1)
apps/desktop/src/lib/trpc/routers/auto-update/index.ts (1)
createAutoUpdateRouter(16-41)
apps/desktop/src/lib/trpc/routers/auto-update/index.ts (2)
apps/desktop/src/main/lib/auto-updater.ts (3)
UpdateDownloadedEvent(12-15)autoUpdateEmitter(17-17)installUpdate(21-23)apps/desktop/src/lib/trpc/index.ts (1)
router(47-47)
⏰ 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 (4)
apps/desktop/src/renderer/screens/main/index.tsx (1)
8-8: LGTM! Clean integration of update notification system.The
useUpdateListenerhook is properly imported and invoked alongside the existinguseAgentHookListener, following the established pattern in this component. The implementation correctly uses tRPC for IPC communication as required by the coding guidelines.Also applies to: 67-67
apps/desktop/src/renderer/components/UpdateToast/index.ts (1)
1-2: LGTM! Proper barrel export pattern.The index file correctly re-exports both the component and hook, following the component organization guidelines.
apps/desktop/src/lib/trpc/routers/index.ts (1)
4-4: LGTM! Standard router integration.The auto-update router is properly imported and registered following the existing pattern used for other routers in the application.
Also applies to: 30-30
apps/desktop/src/lib/trpc/routers/auto-update/index.ts (1)
1-41: LGTM! Excellent implementation following tRPC-electron best practices.The router correctly uses the observable pattern from
@trpc/server/observableas required by the coding guidelines for tRPC subscriptions with trpc-electron. The implementation includes proper event listener cleanup and maintains type safety throughout.Key strengths:
- Observable pattern correctly implemented (lines 18-36)
- Event listeners properly removed in cleanup function (lines 31-34)
- Type-safe event emission with
AutoUpdateEventdiscriminated union- Clean separation of concerns
As per coding guidelines: "For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from
@trpc/server/observableinstead of async generators."
| const openUrl = trpc.external.openUrl.useMutation(); | ||
| const installUpdate = trpc.autoUpdate.installUpdate.useMutation(); | ||
|
|
||
| const handleSeeChanges = () => { | ||
| openUrl.mutate(releaseUrl); | ||
| }; | ||
|
|
||
| const handleRestart = () => { | ||
| installUpdate.mutate(); | ||
| }; |
There was a problem hiding this comment.
Consider adding error handling for the openUrl mutation.
The openUrl mutation (line 21) has no error handling. If opening the release URL fails, the user receives no feedback. While the installUpdate mutation is less critical (as the app will quit if successful), it would improve user experience to handle potential failures for openUrl.
🔎 Proposed error handling
export function UpdateToast({
toastId,
version,
releaseUrl,
}: UpdateToastProps) {
const openUrl = trpc.external.openUrl.useMutation();
const installUpdate = trpc.autoUpdate.installUpdate.useMutation();
const handleSeeChanges = () => {
- openUrl.mutate(releaseUrl);
+ openUrl.mutate(releaseUrl, {
+ onError: (error) => {
+ console.error("Failed to open release URL:", error);
+ toast.error("Failed to open release notes");
+ },
+ });
};
const handleRestart = () => {
installUpdate.mutate();
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx around lines
17 to 26, the openUrl mutation is called without any error handling; update the
handler to handle success and failure by using the mutation's onError/onSuccess
callbacks or awaiting the mutateAsync promise, show user feedback on failure
(e.g., a toast or inline error text) and disable the button while loading to
prevent duplicate clicks; ensure errors are logged (console or telemetry) and
the UI re-enables the control after the operation completes.
| <button | ||
| type="button" | ||
| onClick={handleDismiss} | ||
| className="absolute -top-2 -left-2 size-5 flex items-center justify-center rounded-full bg-background border border-border text-muted-foreground hover:text-foreground transition-colors" | ||
| aria-label="Dismiss" | ||
| > | ||
| <HiMiniXMark className="size-3" /> | ||
| </button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for similar absolute positioning patterns in UI components
rg -n "absolute.*-top.*-left" --type tsx -C 2Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Read the UpdateToast.tsx file to understand the context
cat -n apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxRepository: superset-sh/superset
Length of output: 2071
🏁 Script executed:
#!/bin/bash
# Check what toast library is being used and search for its usage patterns
rg -n "toast|Toast" apps/desktop/src/renderer/components/ --type tsx -l | head -20Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
#!/bin/bash
# Search for dismiss button patterns (tsx type not recognized, try without -t)
rg -n "dismiss|Dismiss" --type ts -A 2 -B 2 | head -50Repository: superset-sh/superset
Length of output: 4159
🏁 Script executed:
#!/bin/bash
# Search for absolute positioning patterns
rg -n "absolute.*-top" -A 1 -B 1 | head -50Repository: superset-sh/superset
Length of output: 965
🏁 Script executed:
#!/bin/bash
# Search for error handling on mutations
rg -n "\.mutate\(" apps/desktop/src/renderer --type ts -A 2 | head -80Repository: superset-sh/superset
Length of output: 6002
🏁 Script executed:
#!/bin/bash
# Search for sonner/toast imports and usage
rg -n "sonner|toast" apps/desktop/src/renderer --type ts -B 2 -A 2 | head -100Repository: superset-sh/superset
Length of output: 11435
🏁 Script executed:
#!/bin/bash
# Check if Button component has dismiss functionality or styling patterns
rg -n "Button" apps/desktop/src/renderer/components/UpdateToast --type ts -B 2 -A 2Repository: superset-sh/superset
Length of output: 1361
🏁 Script executed:
#!/bin/bash
# Look for error handling patterns with onError in mutations
rg -n "onError|\.catch" apps/desktop/src/renderer/components --type ts -B 2 -A 2Repository: superset-sh/superset
Length of output: 46
Add error handling to mutations in UpdateToast component.
The openUrl.mutate() call on line 21 lacks error handling. If opening the release URL fails, the user receives no feedback. Add error handling using either an onError callback in the mutation options or by wrapping with toast.promise() like other parts of the codebase do.
🤖 Prompt for AI Agents
In apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx around lines
21 and 50-57, the openUrl.mutate() call is invoked without error handling so
failures produce no user feedback; update the mutation invocation to include
error handling by either passing an onError callback in the mutate options or
wrapping the call with toast.promise like other code paths: supply success and
error messages for the toast, handle rejected promise paths to show an error
toast, and ensure any cleanup (closing the toast UI) still occurs on error so
the user sees appropriate feedback.
- Fix toast layout with proper container styling and min-width - Use unstyled toast to prevent sonner wrapper interference - Add onDismiss callback to allow re-triggering toast in dev - Add Dev menu (dev-only) with "Simulate Update Available" option 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Change "See changes" button to open the main releases page instead of a specific release tag URL. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add AUTO_UPDATE_STATUS constants (idle/checking/downloading/ready/error) - Show downloading state with version info in toast - Replace X button with "Later" button for dismissing - Change "Restart" to "Install" for clearer action - Add dismiss functionality that hides toast for current session - Emit current status immediately on subscription - Add separate Dev menu items for testing both states 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Avi Peltz <AviPeltz@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx (1)
25-27: Add error handling for theopenUrlmutation.The
openUrl.mutate()call lacks error handling. If opening the release URL fails, users receive no feedback. This was flagged in a previous review but appears unaddressed.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx (1)
7-11: Consider using theAutoUpdateStatustype for stronger type safety.The
statusprop is typed as a string literal union"downloading" | "ready", but the component compares againstAUTO_UPDATE_STATUS.DOWNLOADINGandAUTO_UPDATE_STATUS.READY. While this works, using the exported type would be more maintainable:🔎 Proposed improvement
+import type { AutoUpdateStatus } from "shared/auto-update"; + interface UpdateToastProps { toastId: string | number; - status: "downloading" | "ready"; + status: Extract<AutoUpdateStatus, "downloading" | "ready">; version?: string; }apps/desktop/src/main/lib/auto-updater.ts (1)
18-18: Consider typing the EventEmitter for better type safety.The
autoUpdateEmitteris an untypedEventEmitter. TypeScript's typed event emitter pattern would provide compile-time safety for event names and payloads.🔎 Proposed typed EventEmitter
+import { EventEmitter } from "node:events"; +import type TypedEmitter from "typed-emitter"; + +type AutoUpdateEvents = { + "status-changed": (event: AutoUpdateStatusEvent) => void; +}; + -export const autoUpdateEmitter = new EventEmitter(); +export const autoUpdateEmitter = new EventEmitter() as TypedEmitter<AutoUpdateEvents>;Alternatively, define a simple typed wrapper if you prefer not to add a dependency.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/desktop/src/lib/trpc/routers/auto-update/index.tsapps/desktop/src/main/lib/auto-updater.tsapps/desktop/src/main/lib/local-db/index.tsapps/desktop/src/main/lib/menu.tsapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsxapps/desktop/src/renderer/globals.cssapps/desktop/src/shared/auto-update.tsapps/desktop/src/shared/constants.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/shared/constants.ts
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/main/lib/local-db/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/lib/trpc/routers/auto-update/index.ts
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/src/shared/auto-update.tsapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/main/lib/auto-updater.tsapps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsxapps/desktop/src/main/lib/menu.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/src/shared/auto-update.tsapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/main/lib/auto-updater.tsapps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsxapps/desktop/src/main/lib/menu.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/shared/auto-update.tsapps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/main/lib/auto-updater.tsapps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsxapps/desktop/src/main/lib/menu.ts
**/{components,features}/**/[!.]*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Organize project structure with one folder per component: ComponentName/ComponentName.tsx with index.ts barrel export
Files:
apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsx
**/{components,features}/**/*.{ts,tsx,test.ts,test.tsx,stories.tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate component dependencies (utils, hooks, constants, config, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsx
apps/desktop/src/renderer/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never import Node.js modules in renderer process or shared code - use only in main process (src/main/)
Files:
apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsx
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/main/lib/auto-updater.tsapps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsxapps/desktop/src/main/lib/menu.ts
**/*.{tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use React + TailwindCSS v4 + shadcn/ui for UI development
Files:
apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/renderer/globals.cssapps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsx
**/{components,features}/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
**/{components,features}/**/*.tsx: Nest components in parent's components/ folder if used only once, promote to highest shared parent's components/ if used 2+ times
Use one component per file - do not combine multiple components in a single file
Files:
apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsxapps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsx
apps/desktop/src/main/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Accept object parameters in IPC handlers - do not use positional parameters in ipcMain.handle()
Files:
apps/desktop/src/main/lib/auto-updater.tsapps/desktop/src/main/lib/menu.ts
🧬 Code graph analysis (2)
apps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsx (2)
apps/desktop/src/shared/auto-update.ts (1)
AUTO_UPDATE_STATUS(1-7)apps/desktop/src/renderer/components/UpdateToast/UpdateToast.tsx (1)
UpdateToast(13-82)
apps/desktop/src/main/lib/menu.ts (3)
apps/api/src/env.ts (1)
env(4-33)apps/web/src/env.ts (1)
env(5-53)apps/desktop/src/main/lib/auto-updater.ts (2)
simulateDownloading(120-124)simulateUpdateReady(114-118)
⏰ 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)
apps/desktop/src/renderer/globals.css (1)
254-258: LGTM!The CSS selector correctly scopes the hover animation fix to only update toasts using the
:has()pseudo-class, which is well-supported in Electron's Chromium engine. This ensures other Sonner toasts retain their default behavior.apps/desktop/src/shared/auto-update.ts (1)
1-12: LGTM!Clean implementation of shared constants using
as constfor type safety. The derivedAutoUpdateStatustype pattern is idiomatic TypeScript and ensures type safety across the codebase.apps/desktop/src/main/lib/menu.ts (1)
80-95: LGTM!The Dev menu is properly gated to development mode only, and the simulate functions provide useful testing capabilities for the new update toast flow. The double-guard (menu visibility + function-internal check) is appropriately defensive.
apps/desktop/src/renderer/components/UpdateToast/useUpdateListener.tsx (1)
7-48: LGTM!The hook properly manages toast lifecycle:
- Clears
toastIdRefwhen dismissing (line 21), addressing the previous review concern about blocking subsequent notifications- Dismisses existing toasts before showing new ones to prevent duplicates
- Correctly differentiates between dismissal states (IDLE/CHECKING/ERROR) and display states (DOWNLOADING/READY)
apps/desktop/src/main/lib/auto-updater.ts (1)
126-188: LGTM!The
setupAutoUpdaterfunction is well-structured:
- Proper environment/platform guards
- All relevant electron-updater events are handled with appropriate status emissions
interval.unref()correctly prevents the timer from blocking app exit- Initial update check handles both ready and not-ready app states
When checkForUpdatesInteractive finds no updates, emit IDLE status before showing the dialog. This prevents currentStatus from being stuck at CHECKING, which could cause inconsistent UI state. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
In dev mode, clicking Install on a simulated update would call quitAndInstall with no real update downloaded. Now it just logs a message and dismisses the toast. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/main/lib/auto-updater.ts (1)
127-189: LGTM! Auto-updater lifecycle properly wired to event system.All updater lifecycle events now emit status updates instead of showing dialogs, successfully decoupling the update mechanism from the UI. The event handlers are comprehensive and include appropriate version information where available.
Optional enhancement: The
download-progresshandler (lines 163-167) currently only logs progress. If you want to show a progress bar in the UpdateToast, consider including progress percentage in the status event:🔎 Proposed enhancement to include download progress
Update the
AutoUpdateStatusEventinterface to include progress:export interface AutoUpdateStatusEvent { status: AutoUpdateStatus; version?: string; error?: string; + progress?: number; // 0-100 percentage }Then update the
emitStatusfunction signature and the progress handler:autoUpdater.on("download-progress", (progress) => { console.info( `[auto-updater] Download progress: ${progress.percent.toFixed(1)}%`, ); + autoUpdateEmitter.emit("status-changed", { + status: currentStatus, + version: currentVersion, + progress: progress.percent + }); });
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/main/lib/auto-updater.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/src/main/lib/auto-updater.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/src/main/lib/auto-updater.ts
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/main/lib/auto-updater.ts
apps/desktop/src/main/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Accept object parameters in IPC handlers - do not use positional parameters in ipcMain.handle()
Files:
apps/desktop/src/main/lib/auto-updater.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/main/lib/auto-updater.ts
⏰ 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 (8)
apps/desktop/src/main/lib/auto-updater.ts (8)
1-16: LGTM! Clean event-driven architecture foundation.The imports and interface definition properly support the new event-driven update system. The
AutoUpdateStatusEventinterface is well-typed with appropriate optional fields for version and error information.
24-37: LGTM! Status emission logic is sound.The
emitStatusfunction correctly updates internal state and guards against re-emitting READY status for dismissed updates. The event emission pattern is clean and will integrate well with the tRPC subscription layer.
39-44: LGTM! Status query correctly respects dismissal state.The
getUpdateStatusfunction appropriately returns IDLE when an update has been dismissed, maintaining consistency with the event emission logic.
50-53: LGTM! Dismissal logic is clear and correct.The function properly sets the dismissal flag and immediately notifies subscribers with IDLE status to hide the update toast.
55-65: LGTM! Background update check properly emits status events.The function correctly resets dismissal state for new checks and emits appropriate status events for checking and error states, enabling the UI to track update progress.
67-113: LGTM! Interactive check properly emits IDLE status.The function now correctly emits
IDLEstatus at line 95 when no updates are available, addressing the previous review concern. The combination of status events (for toast UI) and dialogs (for interactive feedback) is appropriate.
115-125: LGTM! Development simulation helpers are well-guarded.The simulation functions are appropriately guarded for development-only use and follow the same dismissal reset pattern as the real update checks. The test version "99.0.0-test" clearly indicates simulated state.
46-48: TheisForceRunAfterparameter inquitAndInstall(false, true)does not control restart behavior.The parameters are
(isSilent, isForceRunAfter). Here,falsecorrectly creates a non-silent installation. However,trueforisForceRunAfteris intended to force the app to run after a silent install completes—but sinceisSilent=false, this parameter is ignored per the electron-updater documentation. The current call will install and restart as expected when users click "Restart" in UpdateToast, but the second parameter serves no purpose here and could be removed for clarity.Likely an incorrect or invalid review comment.
| export const autoUpdateEmitter = new EventEmitter(); | ||
|
|
||
| let currentStatus: AutoUpdateStatus = AUTO_UPDATE_STATUS.IDLE; | ||
| let currentVersion: string | undefined; | ||
| let isDismissed = false; |
There was a problem hiding this comment.
Consider version-specific dismissal tracking.
The isDismissed boolean flag applies to any READY status, regardless of version. If a user dismisses update v1.0.0 and a newer v1.1.0 is downloaded before the next automatic check (4-hour interval), the notification for the newer version will be suppressed.
While unlikely in production (updates rarely release within a 4-hour window), tracking dismissed versions in a Set<string> would be more robust and ensure users are notified about each distinct version.
🔎 Proposed refactor to track dismissed versions
-let isDismissed = false;
+const dismissedVersions = new Set<string>();
function emitStatus(
status: AutoUpdateStatus,
version?: string,
error?: string,
): void {
currentStatus = status;
currentVersion = version;
- if (isDismissed && status === AUTO_UPDATE_STATUS.READY) {
+ if (version && dismissedVersions.has(version) && status === AUTO_UPDATE_STATUS.READY) {
return;
}
autoUpdateEmitter.emit("status-changed", { status, version, error });
}
export function getUpdateStatus(): AutoUpdateStatusEvent {
- if (isDismissed && currentStatus === AUTO_UPDATE_STATUS.READY) {
+ if (currentVersion && dismissedVersions.has(currentVersion) && currentStatus === AUTO_UPDATE_STATUS.READY) {
return { status: AUTO_UPDATE_STATUS.IDLE };
}
return { status: currentStatus, version: currentVersion };
}
export function dismissUpdate(): void {
- isDismissed = true;
+ if (currentVersion) {
+ dismissedVersions.add(currentVersion);
+ }
autoUpdateEmitter.emit("status-changed", { status: AUTO_UPDATE_STATUS.IDLE });
}Then remove the isDismissed = false reset lines from checkForUpdates (line 59), checkForUpdatesInteractive (line 85), and simulation functions (lines 117, 123).
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/desktop/src/main/lib/auto-updater.ts (2)
18-22: Consider typing the EventEmitter for better type safety.The
autoUpdateEmitterlacks type annotations, so subscribers won't get compile-time validation of event names or payload shapes.🔎 Proposed typed EventEmitter
+import type { TypedEventEmitter } from "node:events"; + +type AutoUpdateEvents = { + "status-changed": [AutoUpdateStatusEvent]; +}; + -export const autoUpdateEmitter = new EventEmitter(); +export const autoUpdateEmitter = + new EventEmitter() as TypedEventEmitter<AutoUpdateEvents>;Alternatively, if the Node.js version doesn't support
TypedEventEmitter, you can use a third-party library liketyped-emitteror define a custom typed wrapper.
55-58: Internal state inconsistency when dismissing.
dismissUpdateemitsIDLEdirectly but doesn't updatecurrentStatusviaemitStatus. WhilegetUpdateStatushandles this correctly due to theisDismissedcheck, the internal state diverges from what's emitted. This could confuse future maintainers or cause issues if the logic evolves.🔎 Proposed fix using emitStatus
export function dismissUpdate(): void { isDismissed = true; - autoUpdateEmitter.emit("status-changed", { status: AUTO_UPDATE_STATUS.IDLE }); + currentStatus = AUTO_UPDATE_STATUS.IDLE; + currentVersion = undefined; + autoUpdateEmitter.emit("status-changed", { status: AUTO_UPDATE_STATUS.IDLE }); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/main/lib/auto-updater.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid using any type in TypeScript - maintain type safety unless absolutely necessary
Files:
apps/desktop/src/main/lib/auto-updater.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run Biome for formatting, linting, import organization, and safe fixes at the root level using bun run lint:fix
Files:
apps/desktop/src/main/lib/auto-updater.ts
apps/desktop/src/{main,renderer,preload}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use type-safe IPC communication - define channel types in apps/desktop/src/shared/ipc-channels.ts before implementing handlers
Files:
apps/desktop/src/main/lib/auto-updater.ts
apps/desktop/src/main/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Accept object parameters in IPC handlers - do not use positional parameters in ipcMain.handle()
Files:
apps/desktop/src/main/lib/auto-updater.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined insrc/lib/trpc
Use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from@trpc/server/observableinstead of async generators, as the library explicitly checksisObservable(result)and throws an error otherwise
Files:
apps/desktop/src/main/lib/auto-updater.ts
🧬 Code graph analysis (1)
apps/desktop/src/main/lib/auto-updater.ts (3)
apps/desktop/src/shared/auto-update.ts (2)
AutoUpdateStatus(9-10)AUTO_UPDATE_STATUS(1-7)apps/desktop/src/main/env.main.ts (1)
env(12-44)apps/desktop/src/shared/constants.ts (1)
PLATFORM(4-8)
⏰ 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 (8)
apps/desktop/src/main/lib/auto-updater.ts (8)
1-16: LGTM!Imports and type definitions are well-structured. Good use of the shared
AUTO_UPDATE_STATUSconstants andAutoUpdateStatustype for consistency across the codebase.
24-37: LGTM!The
emitStatusfunction correctly centralizes status management and suppresses redundant READY notifications when dismissed. The guard at lines 32-34 ensures UI consistency.
39-53: LGTM!Both functions are well-implemented:
getUpdateStatuscorrectly masks dismissed READY states as IDLEinstallUpdatesafely handles dev mode and uses appropriatequitAndInstallparameters (isSilent: falseto show installer,isForceRunAfter: trueto restart)
60-70: LGTM!The function correctly guards against dev mode and non-macOS platforms, resets dismissal state, and properly handles errors by emitting an ERROR status with the error message.
72-118: LGTM!Comprehensive interactive check with proper user feedback. The
IDLEstatus emission at line 100 correctly addresses the previous review concern about status consistency when no updates are available.
120-130: LGTM!Development-only simulation functions with proper environment guards. These enable testing the update toast UI without requiring actual update events.
168-172: Download progress is logged but not emitted to the UI.The
download-progresshandler only logs but doesn't emit the progress percentage. If theUpdateToastcomponent or future UI needs to display a progress indicator, this information isn't available via the status system.This may be intentional if the UI only shows a static "downloading" state, but worth confirming.
174-194: LGTM!The
update-downloadedhandler correctly emits the READY status with version info. The interval setup with.unref()ensures the timer won't prevent clean app shutdown, and the app-ready handling covers both immediate and deferred initialization paths.
- Add error status handling to UpdateToast component - Show "Update failed" with error message in toast UI - Add simulateError function for dev testing - Add "Simulate Update Error" option to Dev menu 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 Preview resources have been processed for cleanup |
1 participant
Summary
subscribesubscription andinstallUpdatemutationUpdateToastcomponent with "See changes" and "Restart" buttonsBefore
Native Electron dialog that blocks interaction and feels disruptive.
After
Subtle toast notification that:
Test plan
update-downloadedevent)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.