diff --git a/src/api/services/notification.test.ts b/src/api/services/notification.test.ts index 8539ce04cb..097239e8df 100644 --- a/src/api/services/notification.test.ts +++ b/src/api/services/notification.test.ts @@ -120,6 +120,41 @@ describe("notification service", () => { }); }); + it("suppresses completion delivery while the session already has attention", async () => { + await expect( + notifyTaskCompletion("Done", settings, { + context: { + sessionId: "active-session", + background: false, + }, + }) + ).resolves.toEqual({ + disposition: "suppressed", + systemNotificationSent: false, + soundPlayed: false, + reason: "foreground-session", + }); + + expect(mocks.sendNotification).not.toHaveBeenCalled(); + expect(mocks.playNotificationSound).not.toHaveBeenCalled(); + }); + + it("delivers completion sound once the session is in the background", async () => { + await expect( + notifyTaskCompletion("Done", settings, { + context: { + sessionId: "background-session", + background: true, + }, + }) + ).resolves.toMatchObject({ + disposition: "delivered", + soundPlayed: true, + }); + + expect(mocks.playNotificationSound).toHaveBeenCalledOnce(); + }); + it("uses the selected preset for the test notification", async () => { await sendTestNotification(settings); diff --git a/src/api/services/notificationPolicy.test.ts b/src/api/services/notificationPolicy.test.ts index 92ea307bb3..07a76e81cc 100644 --- a/src/api/services/notificationPolicy.test.ts +++ b/src/api/services/notificationPolicy.test.ts @@ -143,7 +143,7 @@ describe("evaluateNotificationPolicy", () => { ).toBe("deliver"); }); - it("plays foreground completion sound without a redundant system alert", () => { + it("suppresses completion alerts for an attended foreground session", () => { expect( evaluateNotificationPolicy( { @@ -154,12 +154,49 @@ describe("evaluateNotificationPolicy", () => { makeSettings() ) ).toEqual({ - disposition: "deliver", + disposition: "suppress", sendSystemNotification: false, + playSound: false, + reason: "foreground-session", + }); + }); + + it("delivers completion alerts once the session needs background attention", () => { + expect( + evaluateNotificationPolicy( + { + category: "taskCompletion", + context: { sessionId: "background-session", background: true }, + playSound: true, + }, + makeSettings() + ) + ).toEqual({ + disposition: "deliver", + sendSystemNotification: true, playSound: true, }); }); + it("keeps approval and error alerts eligible in an attended session", () => { + for (const category of ["agentApproval", "errors"] as const) { + expect( + evaluateNotificationPolicy( + { + category, + context: { sessionId: "active-session", background: false }, + playSound: true, + }, + makeSettings() + ) + ).toEqual({ + disposition: "deliver", + sendSystemNotification: false, + playSound: true, + }); + } + }); + it("defers background completion during quiet hours", () => { const decision = evaluateNotificationPolicy( { diff --git a/src/api/services/notificationPolicy.ts b/src/api/services/notificationPolicy.ts index 4b28e2db7f..eb3cabc0c2 100644 --- a/src/api/services/notificationPolicy.ts +++ b/src/api/services/notificationPolicy.ts @@ -99,6 +99,7 @@ export interface NotificationPolicyDecision { | "disabled" | "category-disabled" | "critical-only" + | "foreground-session" | "quiet-hours" | "session-muted"; } @@ -249,6 +250,20 @@ export function evaluateNotificationPolicy( }; } + // A completion cue is only useful when the user is no longer attending the + // session. Approval and error alerts remain eligible in the foreground. + if ( + request.category === "taskCompletion" && + request.context?.background === false + ) { + return { + disposition: "suppress", + sendSystemNotification: false, + playSound: false, + reason: "foreground-session", + }; + } + if (!isQuietHoursActive(settings, now)) { return deliver(request.playSound); } diff --git a/src/hooks/cliSession/useBackgroundSessionMonitor.ts b/src/hooks/cliSession/useBackgroundSessionMonitor.ts index d4990255b7..d07c20e4ba 100644 --- a/src/hooks/cliSession/useBackgroundSessionMonitor.ts +++ b/src/hooks/cliSession/useBackgroundSessionMonitor.ts @@ -23,7 +23,7 @@ import { import { registerNotificationSoundUnlock } from "@src/api/services/notificationSound"; import Message from "@src/components/Message"; import { deliverSessionTerminalNotification } from "@src/hooks/session/sessionTerminalNotifications"; -import { sessionByIdAtom } from "@src/store/session"; +import { activeSessionIdAtom, sessionByIdAtom } from "@src/store/session"; import { type NotificationSettings, notificationSettingsAtom, @@ -181,12 +181,15 @@ function deliverCliStatus( t: TFunction, completedTurn: boolean ): void { - const session = isStoreInitialized() - ? getInstrumentedStore().get(sessionByIdAtom(msg.session_id)) - : undefined; + const store = isStoreInitialized() ? getInstrumentedStore() : null; + const session = store?.get(sessionByIdAtom(msg.session_id)); + const activeSessionId = store?.get(activeSessionIdAtom); const sessionInBackground = msg.background ?? session?.background ?? false; + const outsideActiveSession = + sessionInBackground || + (store !== null && activeSessionId !== msg.session_id); const attentionRequired = - isNotificationAttentionRequired(sessionInBackground); + isNotificationAttentionRequired(outsideActiveSession); const sessionName = msg.session_name || session?.name || t("notifications.backgroundSession"); diff --git a/src/types/ui/notification.ts b/src/types/ui/notification.ts index 423da5e069..bac76d6888 100644 --- a/src/types/ui/notification.ts +++ b/src/types/ui/notification.ts @@ -46,6 +46,7 @@ export interface NotificationDeliveryResult { | "category-disabled" | "critical-only" | "duplicate" + | "foreground-session" | "non-primary-window" | "quiet-hours" | "session-muted";