diff --git a/src/api/services/notification.test.ts b/src/api/services/notification.test.ts index 8539ce04c..097239e8d 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 92ea307bb..07a76e81c 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 4b28e2db7..eb3cabc0c 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 d4990255b..d07c20e4b 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/hooks/ui/layout/useElementDimensions.test.ts b/src/hooks/ui/layout/useElementDimensions.test.ts new file mode 100644 index 000000000..8a4b0f999 --- /dev/null +++ b/src/hooks/ui/layout/useElementDimensions.test.ts @@ -0,0 +1,103 @@ +// @vitest-environment jsdom +import React, { act, createElement, useRef } from "react"; +import { type Root, createRoot } from "react-dom/client"; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from "vitest"; + +import { useElementDimensions } from "./useElementDimensions"; + +function DimensionProbe(): React.ReactNode { + const elementRef = useRef(null); + const dimensions = useElementDimensions(elementRef); + + // eslint-disable-next-line react-hooks/refs -- createElement is required because Vitest only includes `.test.ts`; this is a normal React ref prop. + return createElement("div", { + ref: elementRef, + "data-testid": "dimension-probe", + "data-dimensions": `${dimensions.width}x${dimensions.height}`, + }); +} + +describe("useElementDimensions", () => { + let container: HTMLDivElement; + let root: Root; + const actEnvironment = globalThis as typeof globalThis & { + IS_REACT_ACT_ENVIRONMENT?: boolean; + }; + + beforeAll(() => { + actEnvironment.IS_REACT_ACT_ENVIRONMENT = true; + }); + + beforeEach(() => { + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + }); + + afterEach(() => { + act(() => root.unmount()); + container.remove(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + afterAll(() => { + Reflect.deleteProperty(actEnvironment, "IS_REACT_ACT_ENVIRONMENT"); + }); + + it("keeps the window resize fallback when ResizeObserver is unavailable", () => { + vi.stubGlobal("ResizeObserver", undefined); + const addWindowListener = vi.spyOn(window, "addEventListener"); + const removeWindowListener = vi.spyOn(window, "removeEventListener"); + + expect(() => { + act(() => root.render(createElement(DimensionProbe))); + }).not.toThrow(); + + expect(addWindowListener).toHaveBeenCalledWith( + "resize", + expect.any(Function) + ); + + act(() => root.unmount()); + root = createRoot(container); + + expect(removeWindowListener).toHaveBeenCalledWith( + "resize", + expect.any(Function) + ); + }); + + it("disconnects the observer when the measured element unmounts", () => { + const observe = vi.fn(); + const disconnect = vi.fn(); + vi.stubGlobal( + "ResizeObserver", + class ResizeObserverMock { + observe = observe; + unobserve = vi.fn(); + disconnect = disconnect; + } + ); + + act(() => root.render(createElement(DimensionProbe))); + + expect(observe).toHaveBeenCalledWith( + container.querySelector('[data-testid="dimension-probe"]') + ); + + act(() => root.unmount()); + root = createRoot(container); + + expect(disconnect).toHaveBeenCalledOnce(); + }); +}); diff --git a/src/hooks/ui/layout/useElementDimensions.ts b/src/hooks/ui/layout/useElementDimensions.ts index f65ba34a1..36d2001ce 100644 --- a/src/hooks/ui/layout/useElementDimensions.ts +++ b/src/hooks/ui/layout/useElementDimensions.ts @@ -122,7 +122,7 @@ export function useElementDimensions( // Set up ResizeObserver for accurate tracking let resizeObserver: ResizeObserver | null = null; - if (element) { + if (element && typeof ResizeObserver !== "undefined") { resizeObserver = new ResizeObserver(measureDimensions); resizeObserver.observe(element); } diff --git a/src/types/ui/notification.ts b/src/types/ui/notification.ts index 423da5e06..bac76d688 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";