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.
This commit is contained in:
Bohdan Triapitsyn
committed by
GitHub
GitHub
parent
a0bdcae54c
commit
b4f50e0a01
@@ -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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user