From c7ce04fb60aff79f880ef1532702c28b81b9e9dc Mon Sep 17 00:00:00 2001 From: Richard Solomou Date: Wed, 20 May 2026 03:50:00 +0000 Subject: [PATCH 1/2] fix(notifications): retain Notification reference so click handler fires MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking a desktop notification was supposed to switch the app to the task it was about (via TaskLinkService → deepLink.onOpenTask → useTaskDeepLink → navigateToTask), but the JS click listener was never firing. On macOS the OS still raised the app on click, so it looked like "focus works but navigation doesn't" — which is exactly how this surfaced after PR #2210 started firing notifications for other tasks while the app was already focused. Root cause: ElectronNotifier.notify() created `new Notification(...)` in a local variable and let it fall out of scope after `.show()`. V8 could GC the JS wrapper (and the `click` listener with it) before the user clicked. Fix: hold a reference in an instance Set until the notification is clicked or closed, then release it. No behaviour change beyond keeping the wrapper alive long enough for its events to reach JS. Generated-By: PostHog Code Task-Id: ed71008c-f6e9-4cb8-ad76-006037bcce5b --- .../src/main/platform-adapters/electron-notifier.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/apps/code/src/main/platform-adapters/electron-notifier.ts b/apps/code/src/main/platform-adapters/electron-notifier.ts index 25749f459e..21395f7db0 100644 --- a/apps/code/src/main/platform-adapters/electron-notifier.ts +++ b/apps/code/src/main/platform-adapters/electron-notifier.ts @@ -6,6 +6,13 @@ import type { ElectronMainWindow } from "./electron-main-window"; @injectable() export class ElectronNotifier implements INotifier { + // Retain shown notifications so V8 doesn't GC the JS wrapper (and its + // `click` listener) before the user interacts. Without this, the OS still + // shows the notification and macOS will even focus the app on click, but + // the JS click handler never fires — so any in-app routing tied to it + // (e.g. switching to the task the notification was about) silently breaks. + private readonly active = new Set(); + constructor( @inject(MAIN_TOKENS.MainWindow) private readonly mainWindow: ElectronMainWindow, @@ -21,6 +28,10 @@ export class ElectronNotifier implements INotifier { body: options.body, silent: options.silent, }); + this.active.add(notification); + const release = () => this.active.delete(notification); + notification.once("close", release); + notification.once("click", release); if (options.onClick) { notification.on("click", options.onClick); } From b7e47126a473353e5b65baa97ff5de2726c04c3d Mon Sep 17 00:00:00 2001 From: Richard Solomou Date: Wed, 20 May 2026 04:09:54 +0000 Subject: [PATCH 2/2] fix(notifications): also release retained reference on `failed` event MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cover the path where Electron emits `failed` (display failure) instead of `close` / `click`, so the active Set doesn't accumulate references to notifications that were never successfully shown. Same applies to platforms (notably Windows) where OS-timeout dismissal doesn't fire `close` — `failed` is the closest signal we have for that. Generated-By: PostHog Code Task-Id: ed71008c-f6e9-4cb8-ad76-006037bcce5b --- apps/code/src/main/platform-adapters/electron-notifier.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/code/src/main/platform-adapters/electron-notifier.ts b/apps/code/src/main/platform-adapters/electron-notifier.ts index 21395f7db0..84239522f2 100644 --- a/apps/code/src/main/platform-adapters/electron-notifier.ts +++ b/apps/code/src/main/platform-adapters/electron-notifier.ts @@ -32,6 +32,7 @@ export class ElectronNotifier implements INotifier { const release = () => this.active.delete(notification); notification.once("close", release); notification.once("click", release); + notification.once("failed", release); if (options.onClick) { notification.on("click", options.onClick); }