diff --git a/server/index.ts b/server/index.ts index 15f278122..459ec7177 100644 --- a/server/index.ts +++ b/server/index.ts @@ -983,7 +983,9 @@ bus.subscribe((event: RuntimeEvent) => { if (store.bot(bot.id)?.activity !== "dead") store.setActivity(bot.id, "idle"); store.patchBot(bot.id, { unread: true }); if (routineRun?.status !== "failed") { - notify(buildNotification("done", bot, event.threadId, reply)); + // the frame carries the bot's avatar so every desktop client can + // show the notification under that bot's own face + notify(buildNotification("done", bot, event.threadId, reply, { avatarUrl: bot.avatarUrl })); } if (screenPollers.has(bot.id)) { // the last live frame becomes a settled inline screen message — diff --git a/server/notify.test.ts b/server/notify.test.ts index 69ef7e9a0..002aa7a81 100644 --- a/server/notify.test.ts +++ b/server/notify.test.ts @@ -42,6 +42,15 @@ describe("buildNotification", () => { // that conversation, not whatever the bot happens to be showing expect(buildNotification("done", bot, "other-thread", "done")?.threadId).toBe("other-thread"); }); + + it("carries the bot's avatar when one is given", () => { + const avatarUrl = "/api/attachments/123e4567-e89b-12d3-a456-426614174000.webp"; + const frame = buildNotification("done", bot, "thread-1", "pushed the branch", { avatarUrl }); + expect(frame).toMatchObject({ botId: "bot-1", body: "pushed the branch", avatarUrl }); + + // no profile image → the frame stays exactly as before + expect(buildNotification("done", bot, "thread-1", "pushed")?.avatarUrl).toBeUndefined(); + }); }); describe("summarize", () => { diff --git a/server/notify.ts b/server/notify.ts index cebea1e82..21b312e53 100644 --- a/server/notify.ts +++ b/server/notify.ts @@ -19,6 +19,9 @@ export interface Notification { threadId: string; title: string; body: string; + /** The bot's stored profile image, when it has one; clients show it as + * the OS notification's icon so every banner carries its bot's face. */ + avatarUrl?: string; } /** One line, short enough for a lock screen, with the newlines and code @@ -44,6 +47,7 @@ export function buildNotification( bot: NotifyBot, threadId: string, detail: string, + extra?: { avatarUrl?: string }, ): Notification | null { // The toggle means what it says: off is off, including for approvals. // A bot whose notifications you turned off can still block waiting for @@ -66,5 +70,5 @@ export function buildNotification( // badge in the sidebar already carries that much. if (kind === "done" && !body) return null; - return { kind, botId: bot.id, botName: bot.name, threadId, title, body }; + return { kind, botId: bot.id, botName: bot.name, threadId, title, body, ...extra }; } diff --git a/src/lib/notify.test.ts b/src/lib/notify.test.ts index 9471074ee..e75d3f76a 100644 --- a/src/lib/notify.test.ts +++ b/src/lib/notify.test.ts @@ -1,6 +1,11 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { requestNotificationPermission, showNotification, type NotifyFrame } from "./notify"; +import { + buildNotificationOptions, + requestNotificationPermission, + showNotification, + type NotifyFrame, +} from "./notify"; const frame: NotifyFrame = { kind: "done", @@ -48,7 +53,7 @@ describe("desktop notifications", () => { const { notices } = installNotification("granted"); showNotification(frame, vi.fn()); expect(notices).toHaveLength(1); - expect(notices[0]).toMatchObject({ title: frame.title, options: { body: frame.body, tag: frame.threadId } }); + expect(notices[0]).toMatchObject({ title: frame.title, options: { body: frame.body, tag: `openmausbot:${frame.botId}` } }); }); it("opens the exact detached task carried by the notification", () => { @@ -65,4 +70,41 @@ describe("desktop notifications", () => { threadId: "detached-routine-thread", }); }); + + it("groups under the bot, not the thread", () => { + const { notices } = installNotification("granted"); + + showNotification(frame, vi.fn()); + showNotification( + { ...frame, threadId: "thread-2", body: "Second task done" }, + vi.fn(), + ); + + // one bot across two threads shares a tag, so the platform replaces + // rather than stacks; another bot gets its own key + expect(notices[0]?.options?.tag).toBe(`openmausbot:${frame.botId}`); + expect(notices[1]?.options?.tag).toBe(`openmausbot:${frame.botId}`); + showNotification({ ...frame, botId: "bot-2" }, vi.fn()); + expect(notices[2]?.options?.tag).toBe(`openmausbot:bot-2`); + }); + + it("carries the bot's avatar when its profile has one", () => { + const { notices } = installNotification("granted"); + const avatarUrl = "/api/attachments/123e4567-e89b-12d3-a456-426614174000.png"; + + showNotification(frame, vi.fn(), avatarUrl); + expect(notices[0]?.options?.icon).toBe(avatarUrl); + + showNotification(frame, vi.fn(), null); + expect(notices[1]?.options?.icon).toBeUndefined(); + }); +}); + +describe("buildNotificationOptions", () => { + it("keys coalescing on botId and omits a missing avatar", () => { + expect(buildNotificationOptions({ id: "bot-9" })).toEqual({ + tag: "openmausbot:bot-9", + icon: undefined, + }); + }); }); diff --git a/src/lib/notify.ts b/src/lib/notify.ts index d0c8858d2..6357a950e 100644 --- a/src/lib/notify.ts +++ b/src/lib/notify.ts @@ -14,10 +14,29 @@ export function requestNotificationPermission(): Promise return Notification.requestPermission(); } +/** The identity a notification groups under: one bot, wherever it was + * working. Keyed by bot rather than thread so a single bot running across + * tasks and rooms coalesces into one stack instead of stacking banners. */ +export interface NotificationBotIdentity { + id: string; + avatarUrl?: string | null; +} + +/** Presentation options for one bot's notifications: the stable per-bot + * coalescing key platforms replace on (`tag`) and its avatar, when the + * profile has one. Pure so the grouping rule stays testable on its own. */ +export function buildNotificationOptions(bot: NotificationBotIdentity): NotificationOptions { + return { tag: `openmausbot:${bot.id}`, icon: bot.avatarUrl ?? undefined }; +} + /** Show one, unless the app is already in front of the user — a banner over * the window you are looking at is noise, and the chat itself already shows * the card. */ -export function showNotification(frame: NotifyFrame, onOpen: (target: NotificationTarget) => void) { +export function showNotification( + frame: NotifyFrame, + onOpen: (target: NotificationTarget) => void, + avatarUrl?: string | null, +) { if (typeof Notification === "undefined") return; if (document.hasFocus()) return; @@ -27,6 +46,10 @@ export function showNotification(frame: NotifyFrame, onOpen: (target: Notificati }; if (Notification.permission === "granted") { - new Notification(frame.title, { body: frame.body, tag: frame.threadId }).onclick = open; + const options: NotificationOptions = { + body: frame.body, + ...buildNotificationOptions({ id: frame.botId, avatarUrl }), + }; + new Notification(frame.title, options).onclick = open; } } diff --git a/src/state/store.tsx b/src/state/store.tsx index 812d1fd0f..a7f46d067 100644 --- a/src/state/store.tsx +++ b/src/state/store.tsx @@ -1462,7 +1462,11 @@ export function StoreProvider({ children }: { children: ReactNode }) { // unread:false back. Opening a bot from its own notification and // watching the badge return on the next hydration is exactly the // bug that makes notifications feel broken. - showNotification(frame.notification, (target) => openNotificationTarget(dispatch, target, stateRef.current)); + showNotification( + frame.notification, + (target) => openNotificationTarget(dispatch, target, stateRef.current), + stateRef.current.bots.find((bot) => bot.id === frame.notification.botId)?.avatarUrl, + ); break; case "group.deleted": rawDispatch({ type: "groupDeleted", groupId: frame.groupId });