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
This commit is contained in:
@@ -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,
|
||||
},
|
||||
};
|
||||
|
||||
@@ -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,
|
||||
},
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -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) {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user