fix(desktop): retain notifications and activate window on click (#55247)

packages/desktop/src/main/ipc-handlers/window.test.ts

@@ -0,0 +1,61 @@
+import { afterEach, describe, expect, test } from "bun:test"
+import { focusWindow, showWindow } from "../windows/focus"
+
+const originalPlatform = process.platform
+
+afterEach(() => {
+  Object.defineProperty(process, "platform", { value: originalPlatform })
+})
+
+function fakeWindow(minimized: boolean, events: string[]) {
+  return {
+    isMinimized: () => minimized,
+    restore: () => {
+      events.push("win.restore")
+    },
+    show: () => {
+      events.push("win.show")
+    },
+    focus: () => {
+      events.push("win.focus")
+    },
+  }
+}
+
+describe("window show and focus handlers", () => {
+  test.each([
+    { minimized: false, expected: ["win.show"] },
+    { minimized: true, expected: ["win.restore", "win.show"] },
+  ])("showWindow with minimized=$minimized restores before showing", ({ minimized, expected }) => {
+    const events: string[] = []
+    showWindow(fakeWindow(minimized, events))
+    expect(events).toEqual(expected)
+    showWindow(null)
+    expect(events).toEqual(expected)
+  })
+
+  test.each([
+    { platform: "darwin" as const, minimized: false, expected: ["app.focus:steal", "win.focus"] },
+    { platform: "darwin" as const, minimized: true, expected: ["win.restore", "app.focus:steal", "win.focus"] },
+    { platform: "linux" as const, minimized: false, expected: ["win.focus"] },
+    { platform: "linux" as const, minimized: true, expected: ["win.restore", "win.focus"] },
+    { platform: "win32" as const, minimized: true, expected: ["win.restore", "win.focus"] },
+  ])(
+    "focusWindow on $platform with minimized=$minimized restores and activates the app before focusing",
+    ({ platform, minimized, expected }) => {
+      Object.defineProperty(process, "platform", { value: platform })
+      const events: string[] = []
+
+      const app = {
+        focus: (options?: { steal: boolean }) => {
+          events.push(options?.steal ? "app.focus:steal" : "app.focus")
+        },
+      }
+
+      focusWindow(fakeWindow(minimized, events), app)
+      expect(events).toEqual(expected)
+      focusWindow(null, app)
+      expect(events).toEqual(expected)
+    },
+  )
+})

packages/desktop/src/main/ipc-handlers/window.ts

@@ -1,8 +1,9 @@
-import { BrowserWindow } from "electron"
+import { app, BrowserWindow } from "electron"
 import { Effect } from "effect"
 import { WindowRpcs } from "../../shared/ipc-rpc"
 import { IpcPortHandoff } from "../ipc-transport"
 import { getPinchZoomEnabled, setPinchZoomEnabled, setTitlebar, setWindowThemeReady, updateTitlebar } from "../windows"
+import { focusWindow, showWindow } from "../windows/focus"
 import { sender } from "./context"
 
 export const windowHandlers = WindowRpcs.toLayer(
@@ -22,9 +23,9 @@ export const windowHandlers = WindowRpcs.toLayer(
       WindowGetFullscreen: (_args, context) =>
         Effect.sync(() => BrowserWindow.fromWebContents(sender(handoff, context))?.isFullScreen() ?? false),
       WindowSetFocus: (_args, context) =>
-        Effect.sync(() => BrowserWindow.fromWebContents(sender(handoff, context))?.focus()),
+        Effect.sync(() => focusWindow(BrowserWindow.fromWebContents(sender(handoff, context)), app)),
       WindowShow: (_args, context) =>
-        Effect.sync(() => BrowserWindow.fromWebContents(sender(handoff, context))?.show()),
+        Effect.sync(() => showWindow(BrowserWindow.fromWebContents(sender(handoff, context)))),
       WindowGetZoomFactor: (_args, context) => Effect.sync(() => sender(handoff, context).getZoomFactor()),
       WindowSetZoomFactor: ({ factor }, context) =>
         Effect.sync(() => {

packages/desktop/src/main/windows/focus.ts

@@ -0,0 +1,22 @@
+import type { App, BrowserWindow } from "electron"
+
+export type FocusableWindow = Pick<BrowserWindow, "isMinimized" | "restore" | "show" | "focus">
+
+export function showWindow(win: Pick<FocusableWindow, "isMinimized" | "restore" | "show"> | null | undefined) {
+  if (!win) return
+
+  if (win.isMinimized()) win.restore()
+  win.show()
+}
+
+export function focusWindow(
+  win: Pick<FocusableWindow, "isMinimized" | "restore" | "focus"> | null | undefined,
+  app: Pick<App, "focus">,
+) {
+  if (!win) return
+
+  if (win.isMinimized()) win.restore()
+
+  if (process.platform === "darwin") app.focus({ steal: true })
+  win.focus()
+}

packages/desktop/src/renderer/platform/notifications.test.ts

@@ -0,0 +1,129 @@
+import { afterEach, describe, expect, test } from "bun:test"
+import { createDesktopNotify } from "./notifications"
+
+const originalNotification = globalThis.Notification
+
+const created: FakeNotification[] = []
+
+const events: string[] = []
+
+class FakeNotification extends EventTarget implements Notification {
+  static readonly permission: NotificationPermission = "granted"
+
+  static async requestPermission(): Promise<NotificationPermission> {
+    return "granted"
+  }
+
+  onclick: ((this: Notification, ev: Event) => void) | null = null
+  onclose: ((this: Notification, ev: Event) => void) | null = null
+  onerror: ((this: Notification, ev: Event) => void) | null = null
+  onshow: ((this: Notification, ev: Event) => void) | null = null
+
+  readonly body: string
+  readonly data = null
+  readonly dir: NotificationDirection = "auto"
+  readonly icon: string
+  readonly lang = ""
+  readonly silent: boolean | null
+  readonly tag = ""
+  readonly title: string
+
+  constructor(title: string, options?: NotificationOptions) {
+    super()
+    this.title = title
+    this.body = options?.body ?? ""
+    this.icon = options?.icon ?? ""
+    this.silent = options?.silent ?? null
+    events.push(`created:${title}:${this.body}`)
+    created.push(this)
+  }
+
+  close() {
+    events.push("notification.close")
+    this.onclose?.call(this, new Event("close"))
+  }
+}
+
+afterEach(() => {
+  created.length = 0
+  events.length = 0
+  globalThis.Notification = originalNotification
+})
+
+describe("desktop notifications", () => {
+  test("shows and focuses the window on click, runs the callback, and closes the notification", async () => {
+    globalThis.Notification = FakeNotification
+
+    const notify = createDesktopNotify({
+      getWindowFocused: async () => false,
+      showWindow: async () => {
+        events.push("api.showWindow")
+      },
+      setWindowFocus: async () => {
+        events.push("api.setWindowFocus")
+      },
+    })
+
+    await notify("Response ready", "Refactor auth flow", () => {
+      events.push("session.select")
+    })
+
+    const notification = created[0]
+    expect(notification).toBeDefined()
+
+    notification?.onclick?.call(notification, new Event("click"))
+
+    expect(events).toEqual([
+      "created:Response ready:Refactor auth flow",
+      "api.showWindow",
+      "api.setWindowFocus",
+      "session.select",
+      "notification.close",
+    ])
+    expect(notification?.onclick).toBeNull()
+    expect(notification?.onclose).toBeNull()
+    expect(notification?.onerror).toBeNull()
+  })
+
+  test.each([{ event: "close" as const }, { event: "error" as const }])(
+    "releases the Notification handlers when $event fires without a click",
+    async ({ event }) => {
+      globalThis.Notification = FakeNotification
+
+      const notify = createDesktopNotify({
+        getWindowFocused: async () => false,
+        showWindow: async () => {},
+        setWindowFocus: async () => {},
+      })
+
+      await notify("Session error")
+
+      const notification = created[0]
+      expect(notification).toBeDefined()
+
+      if (event === "close") {
+        notification?.onclose?.call(notification, new Event("close"))
+      } else {
+        notification?.onerror?.call(notification, new Event("error"))
+      }
+
+      expect(notification?.onclick).toBeNull()
+      expect(notification?.onclose).toBeNull()
+      expect(notification?.onerror).toBeNull()
+    },
+  )
+
+  test("suppresses desktop notifications while the window is already focused", async () => {
+    globalThis.Notification = FakeNotification
+
+    const notify = createDesktopNotify({
+      getWindowFocused: async () => true,
+      showWindow: async () => {},
+      setWindowFocus: async () => {},
+    })
+
+    await notify("Response ready", "Active session")
+    expect(events).toEqual([])
+    expect(created).toEqual([])
+  })
+})

packages/desktop/src/renderer/platform/notifications.ts

@@ -1,7 +1,13 @@
 import type { Platform } from "@opencode/app/desktop"
 import type { ElectronAPI } from "../api-types"
 
-export function createDesktopNotify(api: ElectronAPI): Platform["notify"] {
+type DesktopNotifyAPI = Pick<ElectronAPI, "getWindowFocused" | "showWindow" | "setWindowFocus">
+
+const MAX_ACTIVE_NOTIFICATIONS = 40
+
+const activeNotifications = new Set<Notification>()
+
+export function createDesktopNotify(api: DesktopNotifyAPI): Platform["notify"] {
   return async (title, description, onClick) => {
     const focused = await api.getWindowFocused().catch(() => document.hasFocus())
 
@@ -13,9 +19,27 @@ export function createDesktopNotify(api: ElectronAPI): Platform["notify"] {
       silent: true,
     })
 
+    if (activeNotifications.size >= MAX_ACTIVE_NOTIFICATIONS) {
+      const oldest = activeNotifications.values().next().value
+
+      if (oldest) activeNotifications.delete(oldest)
+    }
+
+    activeNotifications.add(notification)
+
+    const release = () => {
+      activeNotifications.delete(notification)
+      notification.onclick = null
+      notification.onclose = null
+      notification.onerror = null
+    }
+
+    notification.onclose = release
+    notification.onerror = release
+
     notification.onclick = () => {
-      void api.showWindow()
-      void api.setWindowFocus()
+      release()
+      void Promise.all([api.showWindow(), api.setWindowFocus()])
       onClick?.()
       notification.close()
     }