Conversation
📝 WalkthroughWalkthroughRe-enables meeting-aware gating in OTA toast to suppress update notifications during active/paused sessions and increases the update check interval to 3 minutes. Relocates Notifications rendering from the app shell’s outer fragment into the main content branch without changing gating conditions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App
participant OTA Toast
participant SessionCtx as OngoingSession Context
participant Updater as OTA Update Query
App->>OTA Toast: Render
OTA Toast->>SessionCtx: useOngoingSession()
SessionCtx-->>OTA Toast: { status, sessionId }
alt status in {running_active, running_paused}
OTA Toast-->>App: Skip showing update toast
else
OTA Toast->>Updater: Poll update (refetch every 3 min)
Updater-->>OTA Toast: Update available?
alt Update available
OTA Toast-->>App: Show update toast
else
OTA Toast-->>App: No toast
end
end
sequenceDiagram
autonumber
participant Router as App Router
participant Shell as App Layout
participant Main as Main Content Branch
participant Notifs as Notifications
Router->>Shell: Render app
alt isMain && !isOnboardingNeeded
Shell->>Main: Render EditModeProvider + content
Main->>Notifs: Render Notifications inside main branch
else
Shell-->>Router: No Notifications rendered
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
✨ 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
Status, Documentation and Community
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/components/toast/ota.tsx (1)
104-115: Unify environment checks to use import.meta.env in apps/desktop
Replace allprocess.env.NODE_ENVchecks in the Vite/Tauri desktop app with the correspondingimport.meta.env.PRODorimport.meta.env.DEVflags to align with project-wide convention.• In apps/desktop/src/components/toast/ota.tsx (L107), change
process.env.NODE_ENV === "production"→import.meta.env.PROD
• In apps/desktop/src/routes/__root.tsx (L39, L115, L125), changeprocess.env.NODE_ENV !== "production"/=== "production"→import.meta.env.DEV/import.meta.env.PROD
🧹 Nitpick comments (3)
apps/desktop/src/components/toast/ota.tsx (3)
13-13: Narrow the selector to avoid unnecessary re-renders and unused fields.Only status is used; selecting sessionId causes extra updates.
- const ongoingSession = useOngoingSession((state) => ({ - status: state.status, - sessionId: state.sessionId, - })); + const ongoingStatus = useOngoingSession((s) => s.status);- if (ongoingSession.status === "running_active" || ongoingSession.status === "running_paused") { + if (ongoingStatus === "running_active" || ongoingStatus === "running_paused") {Also applies to: 90-93, 129-131
113-115: Optional: pause polling during meetings to save cycles.If prefetching during meetings isn’t required, gate the query via enabled.
const checkForUpdate = useQuery({ @@ - refetchInterval: 1000 * 60 * 3, + refetchInterval: 1000 * 60 * 3, + enabled: ongoingStatus !== "running_active" && ongoingStatus !== "running_paused", refetchIntervalInBackground: true, });
137-194: DRY: reuse handleUpdateInstall for the Update Now action.Avoid duplicating the install/download UI logic.
- onClick: async () => { - sonnerToast.dismiss("ota-notification"); - const updateChannel = new Channel<number>(); - let totalDownloaded = 0; - let contentLength: number | undefined; - toast({ - id: "update-download", - title: `Downloading Update ${update.version}`, - content: ( - <div className="space-y-1"> - <div>This may take a while...</div> - <DownloadProgress channel={updateChannel} /> - </div> - ), - dismissible: false, - }); - update.downloadAndInstall((progressEvent) => { - if (progressEvent.event === "Started") { - totalDownloaded = 0; - contentLength = progressEvent.data.contentLength; - } else if (progressEvent.event === "Progress") { - totalDownloaded += progressEvent.data.chunkLength; - const totalSize = contentLength || (50 * 1024 * 1024); - const progressPercentage = Math.min(Math.round((totalDownloaded / totalSize) * 100), 99); - updateChannel.onmessage(progressPercentage); - } else if (progressEvent.event === "Finished") { - updateChannel.onmessage(100); - } - }).then(() => { - message("The app will now restart", { kind: "info", title: "Update Installed" }); - setTimeout(relaunch, 2000); - }).catch((err: any) => { - Sentry.captureException(err); - if (!appInApplicationsFolder.data) { - message("Please move the app to the Applications folder and try again", { - kind: "error", - title: "Update Installation Failed", - }); - } else { - message(err, { kind: "error", title: "Update Installation Failed" }); - } - }); - }, + onClick: () => handleUpdateInstall(update, "ota-notification", !!appInApplicationsFolder.data),
📜 Review details
Configuration used: Path: .coderabbit.yaml
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)
apps/desktop/src/components/toast/ota.tsx(2 hunks)apps/desktop/src/routes/app.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
apps/desktop/src/routes/app.tsxapps/desktop/src/components/toast/ota.tsx
🧬 Code graph analysis (1)
apps/desktop/src/routes/app.tsx (1)
apps/desktop/src/components/toast/index.tsx (1)
Notifications(5-13)
⏰ 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: ci (macos, macos-14)
- GitHub Check: ci (windows, windows-latest)
🔇 Additional comments (1)
apps/desktop/src/routes/app.tsx (1)
176-176: LGTM: Notifications moved under OngoingSessionProvider.This ensures the OTA toast can read meeting status via context; gating remains unchanged.
2 participants
@yujonglee
pls take a look