Skip to content

feat(updater): persistent Skip This Version and 24h snooze for update notifications#1980

Open
AndyMik90 wants to merge 2 commits intodevelopfrom
fix/update-notification-snooze
Open

feat(updater): persistent Skip This Version and 24h snooze for update notifications#1980
AndyMik90 wants to merge 2 commits intodevelopfrom
fix/update-notification-snooze

Conversation

@AndyMik90
Copy link
Copy Markdown
Owner

@AndyMik90 AndyMik90 commented Mar 23, 2026

Summary

Adds persistent "Skip This Version" and 24-hour "Remind Me Later" snooze to the update notification system so users aren't nagged repeatedly when they can't or don't want to update.

Fixes #1944

Problem

Users were getting update popups every few minutes with no way to permanently dismiss them. The dismiss was in-memory only (reset on restart), and "Remind Me Later" just closed the dialog with no persistent effect.

Solution

  • Skip This Version — Persists the skipped version to settings.json. That specific version never shows notifications again. A newer version bypasses the skip automatically.
  • Remind Me Later — Snoozes all update notifications for 24 hours. The sidebar banner X-dismiss also snoozes instead of just hiding in-memory.
  • Updates still download silently when suppressed so they're staged and ready when the user wants them.

Changes

  • app-updater.ts — Suppression logic (shouldSuppressUpdate), settings persistence (skipUpdateVersion, snoozeUpdate), gated update-available and update-downloaded IPC events
  • app-update-handlers.ts — 3 new IPC handlers with version format validation
  • app-update-api.ts — Preload API additions (skipAppUpdate, snoozeAppUpdate, isAppUpdateSuppressed)
  • AppUpdateNotification.tsx — "Skip This Version" ghost button + "Remind Me Later" calls snooze
  • UpdateBanner.tsx — Suppression check on mount and poll, X-dismiss calls snooze
  • settings.ts — Added skippedUpdateVersion and updateRemindAfter to AppSettings type
  • ipc.ts — 3 new IPC channel constants + ElectronAPI type declarations
  • settings-mock.ts — Browser mock stubs
  • en/dialogs.json, fr/dialogs.json — i18n key for "Skip This Version"

Testing

  • Biome CI passes
  • TypeScript typecheck passes
  • All 4603 unit tests pass
  • Production build succeeds
  • Pre-PR validation with 9 review agents (security, quality, logic, codebase-fit, test-integrity, i18n, cross-platform)
  • Manual testing on macOS
  • Manual testing on Windows
  • Manual testing on Linux

Cross-Platform Compatibility

All new code uses Electron's app.getPath('userData') via settings-utils.ts for settings persistence — works identically on macOS, Windows, and Linux. No platform-specific code introduced.

Summary by CodeRabbit

  • New Features
    • Skip specific app versions from update notifications—suppression applies permanently until cleared
    • Snooze update notifications for 24 hours before they reappear
    • New "Skip This Version" button and "Remind Later" option in update dialogs

AndyMik90 and others added 2 commits March 23, 2026 14:23
…date notifications

Users were getting nagged with update popups every few minutes with no way
to permanently dismiss them. Now the dialog offers "Skip This Version"
(persists to settings.json, never shows again for that version) and
"Remind Me Later" (24h snooze). The sidebar banner X-dismiss also snoozes
for 24h instead of resetting on restart. Updates still download silently
when suppressed so they're ready when the user wants them.

Closes #1944

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add skippedUpdateVersion/updateRemindAfter to AppSettings type
- Add version format validation on skip IPC handler
- Add suppression check on UpdateBanner mount (downloaded updates)
- Wrap async dismiss/skip/snooze handlers in try/catch
- Export shouldSuppressUpdate directly, remove thin wrapper

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions bot added area/frontend This is frontend only feature New feature or request size/M Medium (100-499 lines) labels Mar 23, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 23, 2026

📝 Walkthrough

Walkthrough

Implemented persistent update suppression via three new operations: users can permanently skip a version, snooze for 24 hours, or check suppression status. Settings now store a skipped version string and snooze expiration timestamp. Update notifications are withheld when suppression is active, with IPC handlers and frontend components wired to enable these controls.

Changes

Cohort / File(s) Summary
Core Update Suppression Logic
apps/desktop/src/main/app-updater.ts
Added shouldSuppressUpdate(), skipUpdateVersion(), snoozeUpdate(), and isUpdateSuppressed() functions. Integrated suppression checks into update-available and update-downloaded event handlers to conditionally suppress notifications and trigger auto-download when appropriate.
IPC Integration
apps/desktop/src/shared/constants/ipc.ts, apps/desktop/src/main/ipc-handlers/app-update-handlers.ts, apps/desktop/src/shared/types/ipc.ts
Added three new IPC channels: APP_UPDATE_SKIP_VERSION, APP_UPDATE_SNOOZE, APP_UPDATE_IS_SUPPRESSED. Registered corresponding handlers with version string validation (non-empty, max 50 chars, allowed characters). Extended ElectronAPI interface with the three new methods returning IPCResult types.
Frontend API & Preload
apps/desktop/src/preload/api/app-update-api.ts
Extended AppUpdateAPI interface with three new methods: skipAppUpdate(), snoozeAppUpdate(), isAppUpdateSuppressed() that forward to the new IPC channels via invokeIpc.
UI Components
apps/desktop/src/renderer/components/AppUpdateNotification.tsx, apps/desktop/src/renderer/components/UpdateBanner.tsx
Added handlers for skip/snooze actions in notification dialog. Conditional "Skip This Version" ghost button renders only before download. Modified dismiss behavior to trigger snooze via snoozeAppUpdate(). Both components now check isAppUpdateSuppressed() before displaying update information.
Settings & Types
apps/desktop/src/shared/types/settings.ts, apps/desktop/src/renderer/lib/mocks/settings-mock.ts
Extended AppSettings interface with skippedUpdateVersion?: string and updateRemindAfter?: number fields. Added mock implementations returning non-suppressed defaults.
Internationalization
apps/desktop/src/shared/i18n/locales/en/dialogs.json, apps/desktop/src/shared/i18n/locales/fr/dialogs.json
Added translation key appUpdate.skipThisVersion ("Skip This Version" in English, "Ignorer cette version" in French).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

area/frontend, size/M

🐰 A hop through the update flow so bright,
Users now can skip or snooze with might,
Suppressed in settings, persistent and true,
No more pesky notifications—hooray for you! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The linked issue #1944 reports an ERR_MODULE_NOT_FOUND error for '@libsql/client' on macOS 2.8.0, but the PR implements update suppression logic unrelated to module resolution or dependency packaging. Review whether this PR actually addresses the root cause of #1944 (module resolution in app.asar) or if it incorrectly closes an unrelated issue; consider reopening #1944 if module fixes are still needed.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main feature: adding persistent Skip This Version and 24-hour snooze functionality for update notifications, which aligns with the primary changes across all modified files.
Out of Scope Changes check ✅ Passed All changes are focused on implementing persistent update suppression (Skip This Version and snooze) through settings, IPC handlers, UI components, and i18n additions, with no unrelated modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/update-notification-snooze

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the user experience around application updates by providing persistent control over update notifications. Users can now choose to permanently skip a specific version or snooze all notifications for 24 hours, preventing repetitive and intrusive prompts. This change addresses a common user pain point by making the update system less disruptive, while still ensuring that new versions are downloaded and available for installation when convenient for the user.

Highlights

  • Persistent Update Dismissal: Implemented a 'Skip This Version' feature that permanently dismisses notifications for a specific update version by saving the preference to settings.json. Newer versions will still trigger notifications.
  • 24-Hour Snooze Option: Added a 'Remind Me Later' option that snoozes all update notifications for 24 hours, persisting this preference across application restarts. The sidebar banner's dismiss action also now triggers this snooze.
  • Silent Background Downloads: Updates will continue to download silently in the background even when notifications are suppressed (skipped or snoozed), ensuring the update is ready for installation when the user chooses.
  • IPC Communication for Update Preferences: Introduced new IPC channels and handlers to allow the renderer process to communicate with the main process for skipping versions, snoozing updates, and checking current suppression status.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@github-actions
Copy link
Copy Markdown
Contributor

Coverage Report for apps/desktop

Status Category Percentage Covered / Total
🔵 Lines 23.31% (🎯 22%) 12412 / 53240
🔵 Statements 22.97% (🎯 22%) 12959 / 56394
🔵 Functions 20.31% (🎯 19%) 2225 / 10952
🔵 Branches 18.05% (🎯 17%) 7088 / 39259
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
apps/desktop/src/main/app-updater.ts 3.85% 2.52% 0% 3.9% 54-227, 232-236, 258-539, 550-755
apps/desktop/src/main/ipc-handlers/app-update-handlers.ts 19.29% 0% 10% 19.29% 40-48, 60-68, 80-88, 100-112, 124-129, 144-152, 164-173, 184-189, 201-205
apps/desktop/src/preload/api/app-update-api.ts 16.66% 100% 11.76% 11.76% 54-65, 72-113
apps/desktop/src/renderer/components/AppUpdateNotification.tsx 0% 0% 0% 0% 21-254
apps/desktop/src/renderer/components/UpdateBanner.tsx 0% 0% 0% 0% 9-289
apps/desktop/src/renderer/lib/mocks/settings-mock.ts 4.16% 100% 0% 4.16% 9-62
apps/desktop/src/shared/constants/ipc.ts 100% 100% 100% 100%
apps/desktop/src/shared/types/ipc.ts 0% 0% 0% 0%
apps/desktop/src/shared/types/settings.ts 0% 0% 0% 0%
Generated in workflow #7833 for commit 1d70d22 by the Vitest Coverage Report Action

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
apps/desktop/src/renderer/components/AppUpdateNotification.tsx (1)

85-98: ⚠️ Potential issue | 🟡 Minor

Consider checking suppression in onAppUpdateAvailable listener.

The onAppUpdateAvailable event listener (lines 87-95) directly shows the dialog without checking if the version is suppressed. If the main process emits an event for a version the user previously skipped (e.g., after an app restart where the main process re-detects the update), the dialog will appear despite the user's preference.

The suppression check in UpdateBanner.tsx handles the polling case, but this push-based notification path could bypass it.

🛠️ Suggested fix
   useEffect(() => {
-    const cleanup = window.electronAPI.onAppUpdateAvailable((info) => {
+    const cleanup = window.electronAPI.onAppUpdateAvailable(async (info) => {
+      // Check if this version is suppressed before showing the dialog
+      try {
+        const suppressed = await window.electronAPI.isAppUpdateSuppressed(info.version);
+        if (suppressed.success && suppressed.data) {
+          return; // Don't show dialog for suppressed versions
+        }
+      } catch {
+        // Continue showing dialog if suppression check fails
+      }
+
       setUpdateInfo(info);
       setIsOpen(true);
       // ... rest of state resets
     });

     return cleanup;
   }, []);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/renderer/components/AppUpdateNotification.tsx` around lines
85 - 98, The onAppUpdateAvailable listener in AppUpdateNotification.tsx should
not unconditionally open the dialog; update the callback registered via
window.electronAPI.onAppUpdateAvailable to first check the stored suppression
state for the incoming info.version (use the same suppression key/mechanism used
by UpdateBanner.tsx, e.g., the suppressed version in localStorage or the
settings accessor). If the version is suppressed, return early and do not call
setUpdateInfo/setIsOpen/setIsDownloading/setIsDownloaded/setDownloadProgress/setDownloadError/setShowReadOnlyWarning;
otherwise proceed to set state as currently implemented. Ensure you reference
the existing listener registration (onAppUpdateAvailable) and the suppression
check used by UpdateBanner.tsx so both paths are consistent.
apps/desktop/src/renderer/components/UpdateBanner.tsx (1)

105-121: ⚠️ Potential issue | 🟡 Minor

Push event listener bypasses suppression check.

The onAppUpdateAvailable listener updates the banner state directly without checking if the version is suppressed. This means a skipped/snoozed version pushed from the main process (e.g., after detecting an update on startup) will still show the banner, even though the polling path (lines 39-45) correctly filters it out.

🛠️ Suggested fix
   useEffect(() => {
-    const cleanup = window.electronAPI.onAppUpdateAvailable((info) => {
+    const cleanup = window.electronAPI.onAppUpdateAvailable(async (info) => {
+      // Check if this version is suppressed before showing banner
+      try {
+        const suppressed = await window.electronAPI.isAppUpdateSuppressed(info.version);
+        if (suppressed.success && suppressed.data) {
+          return; // Don't show banner for suppressed versions
+        }
+      } catch {
+        // Continue showing banner if suppression check fails
+      }
+
       // New update notification - reset dismiss state if new version
       if (currentVersionRef.current !== info.version) {
         setIsDismissed(false);
         currentVersionRef.current = info.version;
       }
       setUpdateInfo(info);
       // ... rest of state resets
     });

     return cleanup;
   }, []);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/renderer/components/UpdateBanner.tsx` around lines 105 -
121, The onAppUpdateAvailable listener currently sets banner state
unconditionally; modify the callback in the useEffect that registers
window.electronAPI.onAppUpdateAvailable to first check whether info.version
matches the suppressed/skipped version (e.g., compare info.version against the
existing suppressedVersion state or suppressedVersionRef.current) and return
early if it is suppressed, otherwise proceed to update setUpdateInfo,
setIsDownloading, setIsDownloaded, setDownloadProgress, setDownloadError and
reset setIsDismissed when the version differs (currentVersionRef.current !==
info.version) as before; ensure you reference the same suppression identifier
used by the polling path so the push path respects the same suppression logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/desktop/src/main/app-updater.ts`:
- Around line 204-210: Remove the unused wrapper function isUpdateSuppressed by
deleting its exported declaration that simply calls shouldSuppressUpdate; ensure
you also remove any export references to isUpdateSuppressed (so only
shouldSuppressUpdate remains public) and run a quick grep to confirm there are
no remaining callers or IPC handlers referencing isUpdateSuppressed before
committing.

In `@apps/desktop/src/main/ipc-handlers/app-update-handlers.ts`:
- Around line 198-208: The APP_UPDATE_IS_SUPPRESSED IPC handler currently
accepts the version string without validation; update the handler
(IPC_CHANNELS.APP_UPDATE_IS_SUPPRESSED) to validate the incoming version format
the same way APP_UPDATE_SKIP_VERSION does before calling shouldSuppressUpdate,
and return a failure IPCResult with a clear error message when format validation
fails; reuse the same validation logic/function used by APP_UPDATE_SKIP_VERSION
to ensure consistency and fail fast on malformed input.

---

Outside diff comments:
In `@apps/desktop/src/renderer/components/AppUpdateNotification.tsx`:
- Around line 85-98: The onAppUpdateAvailable listener in
AppUpdateNotification.tsx should not unconditionally open the dialog; update the
callback registered via window.electronAPI.onAppUpdateAvailable to first check
the stored suppression state for the incoming info.version (use the same
suppression key/mechanism used by UpdateBanner.tsx, e.g., the suppressed version
in localStorage or the settings accessor). If the version is suppressed, return
early and do not call
setUpdateInfo/setIsOpen/setIsDownloading/setIsDownloaded/setDownloadProgress/setDownloadError/setShowReadOnlyWarning;
otherwise proceed to set state as currently implemented. Ensure you reference
the existing listener registration (onAppUpdateAvailable) and the suppression
check used by UpdateBanner.tsx so both paths are consistent.

In `@apps/desktop/src/renderer/components/UpdateBanner.tsx`:
- Around line 105-121: The onAppUpdateAvailable listener currently sets banner
state unconditionally; modify the callback in the useEffect that registers
window.electronAPI.onAppUpdateAvailable to first check whether info.version
matches the suppressed/skipped version (e.g., compare info.version against the
existing suppressedVersion state or suppressedVersionRef.current) and return
early if it is suppressed, otherwise proceed to update setUpdateInfo,
setIsDownloading, setIsDownloaded, setDownloadProgress, setDownloadError and
reset setIsDismissed when the version differs (currentVersionRef.current !==
info.version) as before; ensure you reference the same suppression identifier
used by the polling path so the push path respects the same suppression logic.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 68182f10-720b-46f6-acf4-6bc435f8fb16

📥 Commits

Reviewing files that changed from the base of the PR and between cba7a02 and 1d70d22.

📒 Files selected for processing (11)
  • apps/desktop/src/main/app-updater.ts
  • apps/desktop/src/main/ipc-handlers/app-update-handlers.ts
  • apps/desktop/src/preload/api/app-update-api.ts
  • apps/desktop/src/renderer/components/AppUpdateNotification.tsx
  • apps/desktop/src/renderer/components/UpdateBanner.tsx
  • apps/desktop/src/renderer/lib/mocks/settings-mock.ts
  • apps/desktop/src/shared/constants/ipc.ts
  • apps/desktop/src/shared/i18n/locales/en/dialogs.json
  • apps/desktop/src/shared/i18n/locales/fr/dialogs.json
  • apps/desktop/src/shared/types/ipc.ts
  • apps/desktop/src/shared/types/settings.ts

Comment on lines +204 to +210
/**
* Check if update notifications are currently suppressed for a given version.
* Used by the renderer via IPC to avoid showing stale banners.
*/
export function isUpdateSuppressed(version: string): boolean {
return shouldSuppressUpdate(version);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if isUpdateSuppressed is used anywhere in the codebase
rg -n 'isUpdateSuppressed' --type=ts

Repository: AndyMik90/Aperant

Length of output: 164


Remove the unused isUpdateSuppressed wrapper function.

isUpdateSuppressed is not called anywhere in the codebase—ripgrep found only its definition. It simply wraps shouldSuppressUpdate without adding value. Remove it to reduce API surface.

♻️ Remove unused function
-/**
- * Check if update notifications are currently suppressed for a given version.
- * Used by the renderer via IPC to avoid showing stale banners.
- */
-export function isUpdateSuppressed(version: string): boolean {
-  return shouldSuppressUpdate(version);
-}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/**
* Check if update notifications are currently suppressed for a given version.
* Used by the renderer via IPC to avoid showing stale banners.
*/
export function isUpdateSuppressed(version: string): boolean {
return shouldSuppressUpdate(version);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/main/app-updater.ts` around lines 204 - 210, Remove the
unused wrapper function isUpdateSuppressed by deleting its exported declaration
that simply calls shouldSuppressUpdate; ensure you also remove any export
references to isUpdateSuppressed (so only shouldSuppressUpdate remains public)
and run a quick grep to confirm there are no remaining callers or IPC handlers
referencing isUpdateSuppressed before committing.

Comment on lines +198 to +208
ipcMain.handle(
IPC_CHANNELS.APP_UPDATE_IS_SUPPRESSED,
async (_event, version: string): Promise<IPCResult<boolean>> => {
try {
return { success: true, data: shouldSuppressUpdate(version) };
} catch (error) {
console.error('[app-update-handlers] Check suppression failed:', error);
return { success: false, error: error instanceof Error ? error.message : 'Failed to check suppression' };
}
}
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider adding version validation for consistency.

The APP_UPDATE_IS_SUPPRESSED handler accepts a version string without validation, while APP_UPDATE_SKIP_VERSION (lines 166-168) validates the format. Although shouldSuppressUpdate only performs string comparison (no security risk), adding consistent validation would:

  1. Ensure uniform input handling across related handlers
  2. Fail fast on malformed input rather than silently returning false
♻️ Optional: Add validation for consistency
   ipcMain.handle(
     IPC_CHANNELS.APP_UPDATE_IS_SUPPRESSED,
     async (_event, version: string): Promise<IPCResult<boolean>> => {
       try {
+        if (!version || typeof version !== 'string' || version.length > 50 || !/^[\d\w.-]+$/.test(version)) {
+          return { success: false, error: 'Invalid version format' };
+        }
         return { success: true, data: shouldSuppressUpdate(version) };
       } catch (error) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ipcMain.handle(
IPC_CHANNELS.APP_UPDATE_IS_SUPPRESSED,
async (_event, version: string): Promise<IPCResult<boolean>> => {
try {
return { success: true, data: shouldSuppressUpdate(version) };
} catch (error) {
console.error('[app-update-handlers] Check suppression failed:', error);
return { success: false, error: error instanceof Error ? error.message : 'Failed to check suppression' };
}
}
);
ipcMain.handle(
IPC_CHANNELS.APP_UPDATE_IS_SUPPRESSED,
async (_event, version: string): Promise<IPCResult<boolean>> => {
try {
if (!version || typeof version !== 'string' || version.length > 50 || !/^[\d\w.-]+$/.test(version)) {
return { success: false, error: 'Invalid version format' };
}
return { success: true, data: shouldSuppressUpdate(version) };
} catch (error) {
console.error('[app-update-handlers] Check suppression failed:', error);
return { success: false, error: error instanceof Error ? error.message : 'Failed to check suppression' };
}
}
);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/main/ipc-handlers/app-update-handlers.ts` around lines 198 -
208, The APP_UPDATE_IS_SUPPRESSED IPC handler currently accepts the version
string without validation; update the handler
(IPC_CHANNELS.APP_UPDATE_IS_SUPPRESSED) to validate the incoming version format
the same way APP_UPDATE_SKIP_VERSION does before calling shouldSuppressUpdate,
and return a failure IPCResult with a clear error message when format validation
fails; reuse the same validation logic/function used by APP_UPDATE_SKIP_VERSION
to ensure consistency and fail fast on malformed input.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces persistent "skip" and "snooze" functionality for update notifications, which is a great improvement for user experience. The implementation is solid, touching on the main process, IPC, preload scripts, and renderer components. My review focuses on improving robustness and debuggability. I've suggested safer type handling when reading settings, removing a piece of redundant code, and adding error logging to currently empty catch blocks to aid in future debugging.

Comment on lines +165 to +166
skippedVersion: (settings?.skippedUpdateVersion as string) || null,
remindAfter: (settings?.updateRemindAfter as number) || null,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The type assertions as string and as number are not type-safe. If the settings file gets corrupted or manually edited with incorrect types for skippedUpdateVersion or updateRemindAfter, this could lead to runtime errors. It's safer to use typeof checks to validate the types before using them.

Suggested change
skippedVersion: (settings?.skippedUpdateVersion as string) || null,
remindAfter: (settings?.updateRemindAfter as number) || null,
skippedVersion: typeof settings?.skippedUpdateVersion === 'string' ? settings.skippedUpdateVersion : null,
remindAfter: typeof settings?.updateRemindAfter === 'number' ? settings.updateRemindAfter : null,

Comment on lines +208 to +210
export function isUpdateSuppressed(version: string): boolean {
return shouldSuppressUpdate(version);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The function isUpdateSuppressed is a wrapper around shouldSuppressUpdate and is exported but appears to be unused. The corresponding IPC handler in app-update-handlers.ts calls shouldSuppressUpdate directly. To improve maintainability and reduce redundant code, this function should be removed.

Comment on lines +168 to +186
const handleSkipVersion = async () => {
try {
if (updateInfo) {
await window.electronAPI.skipAppUpdate(updateInfo.version);
}
} catch {
// Skip is best-effort; close the dialog regardless
}
setIsOpen(false);
};

const handleRemindLater = async () => {
try {
await window.electronAPI.snoozeAppUpdate();
} catch {
// Snooze is best-effort; close the dialog regardless
}
setIsOpen(false);
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The catch blocks in handleSkipVersion and handleRemindLater are empty. While the comment explains that the action is "best-effort", swallowing errors completely can make debugging difficult if there's a persistent problem with skipping or snoozing updates. It would be better to log the error to the console.

  const handleSkipVersion = async () => {
    try {
      if (updateInfo) {
        await window.electronAPI.skipAppUpdate(updateInfo.version);
      }
    } catch (error) {
      // Skip is best-effort; close the dialog regardless
      console.error('[AppUpdateNotification] Failed to skip version:', error);
    }
    setIsOpen(false);
  };

  const handleRemindLater = async () => {
    try {
      await window.electronAPI.snoozeAppUpdate();
    } catch (error) {
      // Snooze is best-effort; close the dialog regardless
      console.error('[AppUpdateNotification] Failed to snooze update:', error);
    }
    setIsOpen(false);
  };

Comment on lines +190 to 199
const handleDismiss = async () => {
try {
if (updateInfo) {
await window.electronAPI.snoozeAppUpdate();
}
} catch {
// Snooze is best-effort; dismiss the banner regardless
}
setIsDismissed(true);
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The catch block in handleDismiss is empty. While the comment explains the rationale, swallowing the error makes it difficult to diagnose issues if snoozing updates consistently fails. Please log the error to the console for better debuggability.

Suggested change
const handleDismiss = async () => {
try {
if (updateInfo) {
await window.electronAPI.snoozeAppUpdate();
}
} catch {
// Snooze is best-effort; dismiss the banner regardless
}
setIsDismissed(true);
};
const handleDismiss = async () => {
try {
if (updateInfo) {
await window.electronAPI.snoozeAppUpdate();
}
} catch (error) {
// Snooze is best-effort; dismiss the banner regardless
console.error('[UpdateBanner] Failed to snooze update:', error);
}
setIsDismissed(true);
};

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

setUpdateInfo(null);
return;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Banner snooze becomes permanent within a single session

Medium Severity

After the 24-hour snooze expires, the banner never reappears within the same app session. When handleDismiss fires, it sets isDismissed = true while currentVersionRef.current retains the version string. After snooze expiry, checkForUpdate finds the update is no longer suppressed, but the currentVersionRef.current !== newVersion check is false (same version), so isDismissed is never reset to false. The render guard !updateInfo || isDismissed then permanently hides the banner. The same issue affects the onAppUpdateAvailable push listener. The snooze effectively becomes permanent until restart or a new version, contradicting the intended 24-hour behavior.

Additional Locations (2)
Fix in Cursor Fix in Web

*/
export function isUpdateSuppressed(version: string): boolean {
return shouldSuppressUpdate(version);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exported isUpdateSuppressed function is never imported

Low Severity

isUpdateSuppressed is exported but never imported anywhere in the codebase. The IPC handler for APP_UPDATE_IS_SUPPRESSED imports shouldSuppressUpdate directly instead. This function is a trivial wrapper that adds no value — it's dead code.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/frontend This is frontend only feature New feature or request size/M Medium (100-499 lines)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2.8.0 ERR_MODULE_NOT_FOUND after installing or updating

1 participant