From b41c05161bd7a8cd854d705a64557816975a379b Mon Sep 17 00:00:00 2001 From: CrazyRabbit Date: Fri, 20 Feb 2026 21:14:07 +0200 Subject: [PATCH] fix(session-notification): add grace period to prevent late events from cancelling idle notifications --- src/hooks/session-notification-scheduler.ts | 34 +++++++-- src/hooks/session-notification.test.ts | 84 +++++++++++++++++++-- 2 files changed, 107 insertions(+), 11 deletions(-) diff --git a/src/hooks/session-notification-scheduler.ts b/src/hooks/session-notification-scheduler.ts index d28abd112f..eb47cc30b9 100644 --- a/src/hooks/session-notification-scheduler.ts +++ b/src/hooks/session-notification-scheduler.ts @@ -9,6 +9,8 @@ type SessionNotificationConfig = { idleConfirmationDelay: number skipIfIncompleteTodos: boolean maxTrackedSessions: number + /** Grace period in ms to ignore late-arriving activity events after scheduling (default: 100) */ + activityGracePeriodMs?: number } export function createIdleNotificationScheduler(options: { @@ -24,8 +26,11 @@ export function createIdleNotificationScheduler(options: { const sessionActivitySinceIdle = new Set() const notificationVersions = new Map() const executingNotifications = new Set() + const scheduledAt = new Map() - function cleanupOldSessions(): void { + const activityGracePeriodMs = options.config.activityGracePeriodMs ?? 100 + +function cleanupOldSessions(): void { const maxSessions = options.config.maxTrackedSessions if (notifiedSessions.size > maxSessions) { const sessionsToRemove = Array.from(notifiedSessions).slice(0, notifiedSessions.size - maxSessions) @@ -43,44 +48,58 @@ export function createIdleNotificationScheduler(options: { const sessionsToRemove = Array.from(executingNotifications).slice(0, executingNotifications.size - maxSessions) sessionsToRemove.forEach((id) => executingNotifications.delete(id)) } + if (scheduledAt.size > maxSessions) { + const sessionsToRemove = Array.from(scheduledAt.keys()).slice(0, scheduledAt.size - maxSessions) + sessionsToRemove.forEach((id) => scheduledAt.delete(id)) + } } - function cancelPendingNotification(sessionID: string): void { +function cancelPendingNotification(sessionID: string): void { const timer = pendingTimers.get(sessionID) if (timer) { clearTimeout(timer) pendingTimers.delete(sessionID) } + scheduledAt.delete(sessionID) sessionActivitySinceIdle.add(sessionID) notificationVersions.set(sessionID, (notificationVersions.get(sessionID) ?? 0) + 1) } - function markSessionActivity(sessionID: string): void { +function markSessionActivity(sessionID: string): void { + const scheduledTime = scheduledAt.get(sessionID) + if (scheduledTime && Date.now() - scheduledTime < activityGracePeriodMs) { + return + } + cancelPendingNotification(sessionID) if (!executingNotifications.has(sessionID)) { notifiedSessions.delete(sessionID) } } - async function executeNotification(sessionID: string, version: number): Promise { +async function executeNotification(sessionID: string, version: number): Promise { if (executingNotifications.has(sessionID)) { pendingTimers.delete(sessionID) + scheduledAt.delete(sessionID) return } if (notificationVersions.get(sessionID) !== version) { pendingTimers.delete(sessionID) + scheduledAt.delete(sessionID) return } if (sessionActivitySinceIdle.has(sessionID)) { sessionActivitySinceIdle.delete(sessionID) pendingTimers.delete(sessionID) + scheduledAt.delete(sessionID) return } if (notifiedSessions.has(sessionID)) { pendingTimers.delete(sessionID) + scheduledAt.delete(sessionID) return } @@ -113,6 +132,7 @@ export function createIdleNotificationScheduler(options: { } finally { executingNotifications.delete(sessionID) pendingTimers.delete(sessionID) + scheduledAt.delete(sessionID) if (sessionActivitySinceIdle.has(sessionID)) { notifiedSessions.delete(sessionID) sessionActivitySinceIdle.delete(sessionID) @@ -120,12 +140,13 @@ export function createIdleNotificationScheduler(options: { } } - function scheduleIdleNotification(sessionID: string): void { +function scheduleIdleNotification(sessionID: string): void { if (notifiedSessions.has(sessionID)) return if (pendingTimers.has(sessionID)) return if (executingNotifications.has(sessionID)) return sessionActivitySinceIdle.delete(sessionID) + scheduledAt.set(sessionID, Date.now()) const currentVersion = (notificationVersions.get(sessionID) ?? 0) + 1 notificationVersions.set(sessionID, currentVersion) @@ -138,12 +159,13 @@ export function createIdleNotificationScheduler(options: { cleanupOldSessions() } - function deleteSession(sessionID: string): void { +function deleteSession(sessionID: string): void { cancelPendingNotification(sessionID) notifiedSessions.delete(sessionID) sessionActivitySinceIdle.delete(sessionID) notificationVersions.delete(sessionID) executingNotifications.delete(sessionID) + scheduledAt.delete(sessionID) } return { diff --git a/src/hooks/session-notification.test.ts b/src/hooks/session-notification.test.ts index 2f0377a4c7..04fcf2c25c 100644 --- a/src/hooks/session-notification.test.ts +++ b/src/hooks/session-notification.test.ts @@ -183,14 +183,15 @@ describe("session-notification", () => { expect(notificationCalls).toHaveLength(0) }) - test("should cancel pending notification on session activity", async () => { +test("should cancel pending notification on session activity", async () => { // given - main session is set const mainSessionID = "main-cancel" setMainSession(mainSessionID) const hook = createSessionNotification(createMockPluginInput(), { - idleConfirmationDelay: 100, // Long delay + idleConfirmationDelay: 100, skipIfIncompleteTodos: false, + activityGracePeriodMs: 0, }) // when - session goes idle @@ -258,7 +259,7 @@ describe("session-notification", () => { expect(notificationCalls).toHaveLength(0) }) - test("should mark session activity on message.updated event", async () => { +test("should mark session activity on message.updated event", async () => { // given - main session is set const mainSessionID = "main-message" setMainSession(mainSessionID) @@ -266,6 +267,7 @@ describe("session-notification", () => { const hook = createSessionNotification(createMockPluginInput(), { idleConfirmationDelay: 50, skipIfIncompleteTodos: false, + activityGracePeriodMs: 0, }) // when - session goes idle, then message.updated fires @@ -292,7 +294,7 @@ describe("session-notification", () => { expect(notificationCalls).toHaveLength(0) }) - test("should mark session activity on tool.execute.before event", async () => { +test("should mark session activity on tool.execute.before event", async () => { // given - main session is set const mainSessionID = "main-tool" setMainSession(mainSessionID) @@ -300,6 +302,7 @@ describe("session-notification", () => { const hook = createSessionNotification(createMockPluginInput(), { idleConfirmationDelay: 50, skipIfIncompleteTodos: false, + activityGracePeriodMs: 0, }) // when - session goes idle, then tool.execute.before fires @@ -324,7 +327,7 @@ describe("session-notification", () => { expect(notificationCalls).toHaveLength(0) }) - test("should not send duplicate notification for same session", async () => { +test("should not send duplicate notification for same session", async () => { // given - main session is set const mainSessionID = "main-dup" setMainSession(mainSessionID) @@ -358,4 +361,75 @@ describe("session-notification", () => { // then - only one notification should be sent expect(notificationCalls).toHaveLength(1) }) + + test("should ignore activity events within grace period", async () => { + // given - main session is set + const mainSessionID = "main-grace" + setMainSession(mainSessionID) + + const hook = createSessionNotification(createMockPluginInput(), { + idleConfirmationDelay: 50, + skipIfIncompleteTodos: false, + activityGracePeriodMs: 100, + }) + + // when - session goes idle + await hook({ + event: { + type: "session.idle", + properties: { sessionID: mainSessionID }, + }, + }) + + // when - activity happens immediately (within grace period) + await hook({ + event: { + type: "tool.execute.before", + properties: { sessionID: mainSessionID }, + }, + }) + + // Wait for idle delay to pass + await new Promise((resolve) => setTimeout(resolve, 100)) + + // then - notification SHOULD be sent (activity was within grace period, ignored) + expect(notificationCalls.length).toBeGreaterThanOrEqual(1) + }) + + test("should cancel notification for activity after grace period", async () => { + // given - main session is set + const mainSessionID = "main-grace-cancel" + setMainSession(mainSessionID) + + const hook = createSessionNotification(createMockPluginInput(), { + idleConfirmationDelay: 200, + skipIfIncompleteTodos: false, + activityGracePeriodMs: 50, + }) + + // when - session goes idle + await hook({ + event: { + type: "session.idle", + properties: { sessionID: mainSessionID }, + }, + }) + + // when - wait for grace period to pass + await new Promise((resolve) => setTimeout(resolve, 60)) + + // when - activity happens after grace period + await hook({ + event: { + type: "tool.execute.before", + properties: { sessionID: mainSessionID }, + }, + }) + + // Wait for original delay to pass + await new Promise((resolve) => setTimeout(resolve, 200)) + + // then - notification should NOT be sent (activity cancelled it after grace period) + expect(notificationCalls).toHaveLength(0) + }) })