From 91de51d1a56d3b86530e01262768ff08b01843b7 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Tue, 16 Jun 2026 15:21:27 +0300 Subject: [PATCH] fix: deduplicate desktop notifications and tighten notification text extraction Desktop notifications no longer duplicate when native delivery succeeds Reasoning chain-of-thought is excluded from notification body text Untyped message parts are ignored in notification text extraction --- packages/ui/src/sync/sync-context.tsx | 17 +++--- packages/vscode/webview/main.tsx | 2 +- .../lib/notifications/emitter-runtime.js | 22 +++++--- .../lib/notifications/emitter-runtime.test.js | 47 +++++++++++++++++ .../web/server/lib/notifications/runtime.js | 40 +++++--------- .../lib/notifications/template-runtime.js | 10 +++- .../notifications/template-runtime.test.js | 52 +++++++++++++++++++ 7 files changed, 144 insertions(+), 46 deletions(-) create mode 100644 packages/web/server/lib/notifications/emitter-runtime.test.js diff --git a/packages/ui/src/sync/sync-context.tsx b/packages/ui/src/sync/sync-context.tsx index 473a4043..92736e17 100644 --- a/packages/ui/src/sync/sync-context.tsx +++ b/packages/ui/src/sync/sync-context.tsx @@ -326,11 +326,12 @@ type UiNotificationPayload = { body?: unknown tag?: unknown kind?: unknown - sessionId?: unknown - directory?: unknown - requireHidden?: unknown - desktopStdoutActive?: unknown -} + sessionId?: unknown + directory?: unknown + requireHidden?: unknown + desktopNotificationDelivered?: unknown + desktopStdoutActive?: unknown +} const asOptionalString = (value: unknown): string | undefined => { if (typeof value !== "string") return undefined @@ -349,9 +350,9 @@ const handleUiNotificationEvent = (payload: Event, fallbackDirectory: string): b } const notification = properties as UiNotificationPayload - if (notification.desktopStdoutActive === true && getRuntimeKey() === "local") { - return true - } + if ((notification.desktopNotificationDelivered === true || notification.desktopStdoutActive === true) && getRuntimeKey() === "local") { + return true + } const notifications = getRegisteredRuntimeAPIs()?.notifications if (!notifications?.notifyAgentCompletion) { diff --git a/packages/vscode/webview/main.tsx b/packages/vscode/webview/main.tsx index 2efd5cca..88a2f4c2 100644 --- a/packages/vscode/webview/main.tsx +++ b/packages/vscode/webview/main.tsx @@ -1520,7 +1520,7 @@ const extractNotificationTextFromParts = (parts: unknown): string => { .map((part) => { if (!part || typeof part !== 'object') return ''; const entry = part as { type?: unknown; text?: unknown; content?: unknown }; - if (entry.type === 'text' || typeof entry.text === 'string' || typeof entry.content === 'string') { + if (entry.type === 'text') { return typeof entry.text === 'string' ? entry.text : typeof entry.content === 'string' ? entry.content : ''; } return ''; diff --git a/packages/web/server/lib/notifications/emitter-runtime.js b/packages/web/server/lib/notifications/emitter-runtime.js index ae302912..86b4c7c4 100644 --- a/packages/web/server/lib/notifications/emitter-runtime.js +++ b/packages/web/server/lib/notifications/emitter-runtime.js @@ -29,43 +29,51 @@ export const createNotificationEmitterRuntime = (dependencies) => { const emitDesktopNotification = (payload) => { const desktopNotifyEnabled = getDesktopNotifyEnabled(); if (!desktopNotifyEnabled) { - return; + return false; } if (!payload || typeof payload !== 'object') { - return; + return false; } if (onDesktopNotification) { try { onDesktopNotification(payload); + return true; } catch { // ignore host-side throw } - return; + return false; } try { // stdout fallback for runtimes that parse the one-line `${prefix}{json}` protocol. process.stdout.write(`${desktopNotifyPrefix}${JSON.stringify(payload)}\n`); + return true; } catch { // ignore } + + return false; }; - const broadcastUiNotification = (payload) => { + const broadcastUiNotification = (payload, options = {}) => { const desktopNotifyEnabled = getDesktopNotifyEnabled(); if (!payload || typeof payload !== 'object') { return; } + const desktopNotificationDelivered = options.desktopNotificationDelivered === true; + const syntheticPayload = { type: 'openchamber:notification', properties: { ...payload, - // Tell the UI whether the stdout notification channel is active. - // When true, the desktop UI should skip this SSE notification to avoid duplicates. - // When false, the UI must handle this SSE notification itself. + // Tell local desktop UI whether a native channel already accepted this + // notification. If so, the SSE/WS event is informational only and must + // not create a second OS notification. + desktopNotificationDelivered, + // Legacy marker retained for older clients that only know about stdout. desktopStdoutActive: desktopNotifyEnabled, }, }; diff --git a/packages/web/server/lib/notifications/emitter-runtime.test.js b/packages/web/server/lib/notifications/emitter-runtime.test.js new file mode 100644 index 00000000..a6dd7b77 --- /dev/null +++ b/packages/web/server/lib/notifications/emitter-runtime.test.js @@ -0,0 +1,47 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { createNotificationEmitterRuntime } from './emitter-runtime.js'; + +const createRuntime = (overrides = {}) => createNotificationEmitterRuntime({ + process: { stdout: { write: vi.fn() } }, + getDesktopNotifyEnabled: () => true, + desktopNotifyPrefix: '[desktop-notify]', + getUiNotificationClients: () => new Set(), + getBroadcastGlobalUiEvent: () => null, + ...overrides, +}); + +describe('notification emitter runtime', () => { + it('reports desktop delivery through the injected native callback', () => { + const onDesktopNotification = vi.fn(); + const runtime = createRuntime({ onDesktopNotification }); + const payload = { title: 'Ready', body: 'Done' }; + + expect(runtime.emitDesktopNotification(payload)).toBe(true); + expect(onDesktopNotification).toHaveBeenCalledWith(payload); + }); + + it('reports stdout desktop delivery for legacy shells', () => { + const write = vi.fn(); + const runtime = createRuntime({ process: { stdout: { write } } }); + + expect(runtime.emitDesktopNotification({ title: 'Ready' })).toBe(true); + expect(write).toHaveBeenCalledWith('[desktop-notify]{"title":"Ready"}\n'); + }); + + it('marks UI broadcasts that were already delivered natively', () => { + const broadcastGlobalUiEvent = vi.fn(); + const runtime = createRuntime({ getBroadcastGlobalUiEvent: () => broadcastGlobalUiEvent }); + + runtime.broadcastUiNotification({ title: 'Ready' }, { desktopNotificationDelivered: true }); + + expect(broadcastGlobalUiEvent).toHaveBeenCalledWith({ + type: 'openchamber:notification', + properties: { + title: 'Ready', + desktopNotificationDelivered: true, + desktopStdoutActive: true, + }, + }); + }); +}); diff --git a/packages/web/server/lib/notifications/runtime.js b/packages/web/server/lib/notifications/runtime.js index adce7a66..5a2d8259 100644 --- a/packages/web/server/lib/notifications/runtime.js +++ b/packages/web/server/lib/notifications/runtime.js @@ -279,8 +279,8 @@ export const createNotificationTriggerRuntime = (deps) => { directory: notificationDirectory, requireHidden: settings.notificationMode !== 'always', }; - emitDesktopNotification(notificationPayload); - broadcastUiNotification(notificationPayload); + const desktopNotificationDelivered = emitDesktopNotification(notificationPayload); + broadcastUiNotification(notificationPayload, { desktopNotificationDelivered }); } await sendPushToAllUiSessions( @@ -341,8 +341,8 @@ export const createNotificationTriggerRuntime = (deps) => { directory: notificationDirectory, requireHidden: settings.notificationMode !== 'always', }; - emitDesktopNotification(notificationPayload); - broadcastUiNotification(notificationPayload); + const desktopNotificationDelivered = emitDesktopNotification(notificationPayload); + broadcastUiNotification(notificationPayload, { desktopNotificationDelivered }); } await sendPushToAllUiSessions( @@ -408,7 +408,7 @@ export const createNotificationTriggerRuntime = (deps) => { } if (settings.nativeNotificationsEnabled) { - emitDesktopNotification({ + const notificationPayload = { kind: 'question', title, body, @@ -416,17 +416,9 @@ export const createNotificationTriggerRuntime = (deps) => { sessionId, directory: notificationDirectory, requireHidden: settings.notificationMode !== 'always', - }); - - broadcastUiNotification({ - kind: 'question', - title, - body, - tag: `question-${sessionId}`, - sessionId, - directory: notificationDirectory, - requireHidden: settings.notificationMode !== 'always', - }); + }; + const desktopNotificationDelivered = emitDesktopNotification(notificationPayload); + broadcastUiNotification(notificationPayload, { desktopNotificationDelivered }); } void sendPushToAllUiSessions( @@ -530,7 +522,7 @@ export const createNotificationTriggerRuntime = (deps) => { } if (settings.nativeNotificationsEnabled) { - emitDesktopNotification({ + const notificationPayload = { kind: 'permission', title, body, @@ -538,17 +530,9 @@ export const createNotificationTriggerRuntime = (deps) => { sessionId, directory: notificationDirectory, requireHidden: settings.notificationMode !== 'always', - }); - - broadcastUiNotification({ - kind: 'permission', - title, - body, - tag: requestKey ? `permission-${requestKey}` : `permission-${sessionId}`, - sessionId, - directory: notificationDirectory, - requireHidden: settings.notificationMode !== 'always', - }); + }; + const desktopNotificationDelivered = emitDesktopNotification(notificationPayload); + broadcastUiNotification(notificationPayload, { desktopNotificationDelivered }); } if (requestKey) { diff --git a/packages/web/server/lib/notifications/template-runtime.js b/packages/web/server/lib/notifications/template-runtime.js index 377e18bd..b7233c3e 100644 --- a/packages/web/server/lib/notifications/template-runtime.js +++ b/packages/web/server/lib/notifications/template-runtime.js @@ -84,11 +84,17 @@ export const createNotificationTemplateRuntime = (deps) => { : text; }; + const isNotificationTextPart = (part) => { + if (!part || typeof part !== 'object') return false; + if (part.type !== 'text') return false; + return typeof part.text === 'string' || typeof part.content === 'string'; + }; + const extractTextFromParts = (parts, maxLength = NOTIFICATION_BODY_MAX_CHARS) => { if (!Array.isArray(parts) || parts.length === 0) return ''; const textParts = parts - .filter((part) => part && (part.type === 'text' || typeof part.text === 'string' || typeof part.content === 'string')) + .filter(isNotificationTextPart) .map((part) => part.text || part.content || '') .filter(Boolean); @@ -112,7 +118,7 @@ export const createNotificationTemplateRuntime = (deps) => { const content = info.content; if (Array.isArray(content)) { const textContent = content - .filter((entry) => entry && (entry.type === 'text' || typeof entry.text === 'string')) + .filter(isNotificationTextPart) .map((entry) => entry.text || '') .filter(Boolean); if (textContent.length > 0) { diff --git a/packages/web/server/lib/notifications/template-runtime.test.js b/packages/web/server/lib/notifications/template-runtime.test.js index eab6eab1..8ebd0f69 100644 --- a/packages/web/server/lib/notifications/template-runtime.test.js +++ b/packages/web/server/lib/notifications/template-runtime.test.js @@ -30,3 +30,55 @@ describe('notification template runtime zen models', () => { await expect(runtime.resolveZenModel()).resolves.toBe('trinity-large-preview-free'); }); }); + +describe('notification template message extraction', () => { + afterEach(() => { + globalThis.fetch = originalFetch; + }); + + it('excludes reasoning parts from payload message text', () => { + const runtime = createRuntime(); + + expect(runtime.extractLastMessageText({ + properties: { + info: { + parts: [ + { type: 'reasoning', text: 'private chain of thought' }, + { type: 'text', text: 'final answer' }, + ], + }, + }, + })).toBe('final answer'); + }); + + it('ignores untyped parts even when they contain text', () => { + const runtime = createRuntime(); + + expect(runtime.extractLastMessageText({ + properties: { + info: { + parts: [ + { text: 'untyped text' }, + { content: 'untyped content' }, + { type: 'text', text: 'typed final answer' }, + ], + }, + }, + })).toBe('typed final answer'); + }); + + it('excludes reasoning parts when fetching assistant messages', async () => { + const runtime = createRuntime(); + globalThis.fetch = vi.fn(async () => new Response(JSON.stringify([ + { + info: { id: 'msg-1', role: 'assistant', finish: 'stop' }, + parts: [ + { type: 'reasoning', text: 'private chain of thought' }, + { type: 'text', text: 'final answer' }, + ], + }, + ]))); + + await expect(runtime.fetchLastAssistantMessageText('session-1', 'msg-1')).resolves.toBe('final answer'); + }); +});