From b4f50e0a01b34ceea2ac9b44ff59a54cbede6b12 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sun, 12 Jul 2026 14:47:57 +0300 Subject: [PATCH] fix(notifications): handle subagents and session errors (#2156) Use authoritative session.idle and session.error events for notifications while retaining legacy message.updated handling for compatibility. Classify sessions through targeted, directory-aware session lookups instead of fetching the full session list. Cache confirmed root and parent session relationships without treating failed lookups as root sessions. Honor subagent completion settings and templates across the server-driven web, desktop, and mobile paths, and bring the VS Code webview notification policy to feature parity. Use freshly synchronized VS Code settings, retry failed settings syncs, extract session error messages, and deduplicate authoritative and legacy completion and error events. --- packages/vscode/webview/main.tsx | 108 ++++++++++-------- .../web/server/lib/notifications/runtime.js | 91 +++++++++------ 2 files changed, 118 insertions(+), 81 deletions(-) diff --git a/packages/vscode/webview/main.tsx b/packages/vscode/webview/main.tsx index 2f0f182e..0f372275 100644 --- a/packages/vscode/webview/main.tsx +++ b/packages/vscode/webview/main.tsx @@ -1474,6 +1474,7 @@ onCommand('windowFocusChanged', (payload) => { }); const readyNotificationCooldowns = new Map(); +const errorNotificationCooldowns = new Map(); const READY_NOTIFICATION_COOLDOWN_MS = 5000; const DEFAULT_NOTIFICATION_MESSAGE_MAX_LENGTH = 250; let notificationSettingsSyncPromise: Promise | null = null; @@ -1507,6 +1508,7 @@ const ensureNotificationSettingsSynced = async () => { notificationSettingsSyncPromise = import('@/lib/persistence') .then(({ syncDesktopSettings }) => syncDesktopSettings()) .catch((error) => { + notificationSettingsSyncPromise = null; console.warn('[OpenChamber] Failed to sync notification settings:', error); }); } @@ -1601,7 +1603,7 @@ const fetchLastAssistantMessageText = async (sessionId: string, messageId?: stri const getNotificationTemplate = ( settings: { notificationTemplates?: Record }, - key: 'completion' | 'error' | 'question', + key: 'completion' | 'subtask' | 'error' | 'question', fallback: { title: string; message: string }, ) => { const candidate = settings.notificationTemplates?.[key]; @@ -1635,6 +1637,12 @@ const getNotificationSessionId = (payload: Record): string => { return getPayloadString(info?.sessionID ?? info?.sessionId ?? properties.sessionID ?? properties.sessionId ?? properties.session); }; +const getNotificationDirectory = (payload: Record): string | null => { + const properties = (payload.properties ?? payload) as Record; + const info = properties.info as Record | undefined; + return getPayloadString(properties.directory ?? info?.directory) || null; +}; + window.addEventListener('openchamber:vscode-notification-event', (event) => { const detail = (event as CustomEvent<{ payload?: unknown }>).detail; const payload = detail?.payload; @@ -1655,66 +1663,70 @@ window.addEventListener('openchamber:vscode-notification-event', (event) => { import('@/stores/useUIStore'), import('@/stores/permissionStore'), ]).then(async ([{ useUIStore }, { usePermissionStore }]) => { - const localSettings = useUIStore.getState(); await ensureNotificationSettingsSynced(); - const syncedSettings = useUIStore.getState(); - const settings = { - ...syncedSettings, - nativeNotificationsEnabled: localSettings.nativeNotificationsEnabled, - notificationMode: localSettings.notificationMode, - notifyOnCompletion: localSettings.notifyOnCompletion, - notifyOnError: localSettings.notifyOnError, - notifyOnQuestion: localSettings.notifyOnQuestion, - notificationTemplates: localSettings.notificationTemplates, - summarizeLastMessage: localSettings.summarizeLastMessage, - summaryThreshold: localSettings.summaryThreshold, - summaryLength: localSettings.summaryLength, - maxLastMessageLength: localSettings.maxLastMessageLength, - }; + const settings = useUIStore.getState(); if (!settings.nativeNotificationsEnabled) { return; } const requireHidden = settings.notificationMode !== 'always'; const messageId = getPayloadString(info?.id); - const rawLastMessage = extractNotificationLastMessage(record) || await fetchLastAssistantMessageText(sessionId, messageId); + const error = properties.error; + const errorMessage = getPayloadString( + typeof error === 'object' && error + ? (error as { message?: unknown }).message + : error, + ); + const rawLastMessage = extractNotificationLastMessage(record) + || errorMessage + || await fetchLastAssistantMessageText(sessionId, messageId); const lastMessage = prepareNotificationLastMessage( rawLastMessage, settings, ); const variables = buildNotificationVariables(record, sessionId, lastMessage); - if (type === 'message.updated' && getPayloadString(info?.role) === 'assistant') { - const finish = getPayloadString(info?.finish); - if (finish === 'stop') { - if (!settings.notifyOnCompletion) return; - const now = Date.now(); - const lastAt = readyNotificationCooldowns.get(sessionId) ?? 0; - if (now - lastAt < READY_NOTIFICATION_COOLDOWN_MS) return; - readyNotificationCooldowns.set(sessionId, now); - const template = getNotificationTemplate(settings, 'completion', { title: '{agent_name} is ready', message: '{model_name} completed the task' }); - const title = resolveTemplate(template.title, variables) || 'Agent is ready'; - const body = resolveTemplate(template.message, variables); - showOpenChamberNotification({ - title, - body: shouldApplyTemplateMessage(template.message, body, variables) ? body : `${variables.model_name} completed the task`, - sessionId, - requireHidden, - }); - return; - } + const isAssistantMessage = type === 'message.updated' && getPayloadString(info?.role) === 'assistant'; + const finish = isAssistantMessage ? getPayloadString(info?.finish) : ''; + const isCompletion = type === 'session.idle' || finish === 'stop'; + const isError = type === 'session.error' || finish === 'error'; - if (finish === 'error') { - if (!settings.notifyOnError) return; - const template = getNotificationTemplate(settings, 'error', { title: 'Tool error', message: '{last_message}' }); - const title = resolveTemplate(template.title, variables) || 'Tool error'; - const body = resolveTemplate(template.message, variables); - showOpenChamberNotification({ - title, - body: shouldApplyTemplateMessage(template.message, body, variables) ? body : 'An error occurred', - sessionId, - requireHidden, - }); - } + if (isCompletion) { + const session = await opencodeClient.getSession(sessionId, getNotificationDirectory(record)).catch(() => undefined); + if (!session) return; + const isSubtask = Boolean(session?.parentID); + if (isSubtask ? !settings.notifyOnSubtasks : !settings.notifyOnCompletion) return; + const now = Date.now(); + const lastAt = readyNotificationCooldowns.get(sessionId) ?? 0; + if (now - lastAt < READY_NOTIFICATION_COOLDOWN_MS) return; + readyNotificationCooldowns.set(sessionId, now); + const template = getNotificationTemplate(settings, isSubtask ? 'subtask' : 'completion', { title: '{agent_name} is ready', message: '{model_name} completed the task' }); + const title = resolveTemplate(template.title, variables) || 'Agent is ready'; + const body = resolveTemplate(template.message, variables); + showOpenChamberNotification({ + title, + body: shouldApplyTemplateMessage(template.message, body, variables) ? body : `${variables.model_name} completed the task`, + sessionId, + requireHidden, + }); + return; + } + + if (isError) { + if (!settings.notifyOnError) return; + const now = Date.now(); + const lastAt = errorNotificationCooldowns.get(sessionId) ?? 0; + if (now - lastAt < READY_NOTIFICATION_COOLDOWN_MS) return; + errorNotificationCooldowns.set(sessionId, now); + const template = getNotificationTemplate(settings, 'error', { title: 'Tool error', message: '{last_message}' }); + const title = resolveTemplate(template.title, variables) || 'Tool error'; + const body = resolveTemplate(template.message, variables); + showOpenChamberNotification({ + title, + body: shouldApplyTemplateMessage(template.message, body, variables) ? body : 'An error occurred', + sessionId, + requireHidden, + }); + return; } if (type === 'question.asked') { diff --git a/packages/web/server/lib/notifications/runtime.js b/packages/web/server/lib/notifications/runtime.js index e9078160..e544e147 100644 --- a/packages/web/server/lib/notifications/runtime.js +++ b/packages/web/server/lib/notifications/runtime.js @@ -106,6 +106,7 @@ export const createNotificationTriggerRuntime = (deps) => { const pushPermissionDebounceTimers = new Map(); const notifiedPermissionRequests = new Set(); const lastReadyNotificationAt = new Map(); + const lastErrorNotificationAt = new Map(); const sessionParentIdCache = new Map(); const SESSION_PARENT_CACHE_TTL_MS = 60 * 1000; @@ -132,24 +133,26 @@ export const createNotificationTriggerRuntime = (deps) => { return `/?session=${encodeURIComponent(sessionId)}`; }; - const getCachedSessionParentId = (sessionId) => { - const entry = sessionParentIdCache.get(sessionId); + const getSessionParentCacheKey = (sessionId, directory) => `${directory || ''}\0${sessionId}`; + + const getCachedSessionParentId = (sessionId, directory) => { + const cacheKey = getSessionParentCacheKey(sessionId, directory); + const entry = sessionParentIdCache.get(cacheKey); if (!entry) return undefined; if (Date.now() - entry.at > SESSION_PARENT_CACHE_TTL_MS) { - sessionParentIdCache.delete(sessionId); + sessionParentIdCache.delete(cacheKey); return undefined; } return entry.parentID; }; - const setCachedSessionParentId = (sessionId, parentID) => { - if (!parentID) return; - sessionParentIdCache.set(sessionId, { parentID: parentID ?? null, at: Date.now() }); + const setCachedSessionParentId = (sessionId, directory, parentID) => { + sessionParentIdCache.set(getSessionParentCacheKey(sessionId, directory), { parentID: parentID ?? null, at: Date.now() }); }; const getParentIdFromPayload = (payload) => { - if (!payload || typeof payload !== 'object') return null; - if (payload.type !== 'session.created' && payload.type !== 'session.updated') return null; + if (!payload || typeof payload !== 'object') return undefined; + if (payload.type !== 'session.created' && payload.type !== 'session.updated') return undefined; const parentID = payload.properties?.info?.parentID ?? null; return typeof parentID === 'string' && parentID.length > 0 ? parentID : null; }; @@ -157,20 +160,22 @@ export const createNotificationTriggerRuntime = (deps) => { const maybeCacheSessionParentFromPayload = (payload) => { const sessionId = extractSessionIdFromPayload(payload); if (typeof sessionId !== 'string' || sessionId.length === 0) return; + const directory = extractDirectoryFromPayload(payload); const parentID = getParentIdFromPayload(payload); - if (parentID) { - setCachedSessionParentId(sessionId, parentID); - } + if (parentID === undefined) return; + setCachedSessionParentId(sessionId, directory, parentID); }; - const fetchSessionParentId = async (sessionId) => { + const fetchSessionParentId = async (sessionId, directory) => { if (!sessionId) return undefined; - const cached = getCachedSessionParentId(sessionId); + const cached = getCachedSessionParentId(sessionId, directory); if (cached !== undefined) return cached; try { - const response = await fetch(buildOpenCodeUrl('/session', ''), { + const base = buildOpenCodeUrl(`/session/${encodeURIComponent(sessionId)}`, ''); + const url = directory ? `${base}?directory=${encodeURIComponent(directory)}` : base; + const response = await fetch(url, { method: 'GET', headers: { Accept: 'application/json', @@ -181,21 +186,15 @@ export const createNotificationTriggerRuntime = (deps) => { if (!response.ok) { return undefined; } - const data = await response.json().catch(() => null); - const sessions = Array.isArray(data) - ? data - : Array.isArray(data?.items) - ? data.items - : Array.isArray(data?.data) - ? data.data - : null; - if (!sessions) { + const session = await response.json().catch(() => null); + if (!session || typeof session !== 'object') { return undefined; } - const match = sessions.find((session) => session && typeof session === 'object' && session.id === sessionId); - const parentID = match?.parentID ?? null; - setCachedSessionParentId(sessionId, parentID); + const parentID = typeof session.parentID === 'string' && session.parentID.length > 0 + ? session.parentID + : null; + setCachedSessionParentId(sessionId, directory, parentID); return parentID; } catch { return undefined; @@ -204,14 +203,14 @@ export const createNotificationTriggerRuntime = (deps) => { // Mirrors client-side autoRespondsPermission: a session auto-accepts if it // OR any ancestor is flagged. Walks the parent chain via fetchSessionParentId. - const isSessionAutoAccepting = async (sessionId) => { + const isSessionAutoAccepting = async (sessionId, directory) => { if (!sessionId || autoAcceptingSessions.size === 0) return false; let current = sessionId; const seen = new Set(); while (current && !seen.has(current)) { if (autoAcceptingSessions.has(current)) return true; seen.add(current); - const parent = await fetchSessionParentId(current); + const parent = await fetchSessionParentId(current, directory); if (!parent) return false; current = parent; } @@ -306,6 +305,27 @@ export const createNotificationTriggerRuntime = (deps) => { const sessionId = extractSessionIdFromPayload(payload); const notificationDirectory = extractDirectoryFromPayload(payload); + if ((payload.type === 'session.idle' || payload.type === 'session.error') && sessionId) { + const error = payload.properties?.error; + const errorText = typeof error?.message === 'string' + ? error.message + : typeof error === 'string' ? error : ''; + await maybeSendPushForTrigger({ + ...payload, + type: 'message.updated', + properties: { + ...payload.properties, + info: { + sessionID: sessionId, + role: 'assistant', + finish: payload.type === 'session.error' ? 'error' : 'stop', + ...(errorText ? { parts: [{ type: 'text', text: errorText }] } : {}), + }, + }, + }); + return; + } + if (payload.type === 'message.updated') { const info = payload.properties?.info; if (info?.role === 'assistant' && info?.finish === 'stop' && sessionId) { @@ -315,9 +335,9 @@ export const createNotificationTriggerRuntime = (deps) => { const parentIDFromPayload = getParentIdFromPayload(payload); const parentID = parentIDFromPayload ? parentIDFromPayload - : await fetchSessionParentId(sessionId); + : await fetchSessionParentId(sessionId, notificationDirectory); - if (parentID) { + if (parentID !== null) { return; } } @@ -350,7 +370,7 @@ export const createNotificationTriggerRuntime = (deps) => { try { const templates = settings.notificationTemplates || {}; - const isSubtask = await fetchSessionParentId(sessionId); + const isSubtask = await fetchSessionParentId(sessionId, notificationDirectory); const completionTemplate = isSubtask && settings.notifyOnSubtasks !== false ? (templates.subtask || templates.completion || { title: '{agent_name} is ready', message: '{model_name} completed the task' }) : (templates.completion || { title: '{agent_name} is ready', message: '{model_name} completed the task' }); @@ -411,6 +431,11 @@ export const createNotificationTriggerRuntime = (deps) => { const settings = await readSettingsFromDisk(); if (settings.notifyOnError === false) return; + const now = Date.now(); + const lastAt = lastErrorNotificationAt.get(sessionId) ?? 0; + if (now - lastAt < PUSH_READY_COOLDOWN_MS) return; + lastErrorNotificationAt.set(sessionId, now); + if (settings.notificationMode !== 'always' && getIsWindowFocused?.()) { return; } @@ -584,7 +609,7 @@ export const createNotificationTriggerRuntime = (deps) => { // Client may be in Permission Auto-Accept for this session (or any // ancestor). Skip the whole notification path — the client responds // directly and the user has opted out of approval prompts. - if (await isSessionAutoAccepting(sessionId)) { + if (await isSessionAutoAccepting(sessionId, notificationDirectory)) { if (requestKey) notifiedPermissionRequests.add(requestKey); return; } @@ -597,7 +622,7 @@ export const createNotificationTriggerRuntime = (deps) => { const timer = setTimeout(async () => { pushPermissionDebounceTimers.delete(sessionId); - if (await isSessionAutoAccepting(sessionId)) { + if (await isSessionAutoAccepting(sessionId, notificationDirectory)) { if (requestKey) notifiedPermissionRequests.add(requestKey); return; }