Merge pull request #2376 from acamq/fix/idle-notification-grace-period

fix(session-notification): add grace period to prevent late events from cancelling idle notifications

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,6 +26,9 @@ export function createIdleNotificationScheduler(options: {
   const sessionActivitySinceIdle = new Set<string>()
   const notificationVersions = new Map<string, number>()
   const executingNotifications = new Set<string>()
+  const scheduledAt = new Map<string, number>()
+
+  const activityGracePeriodMs = options.config.activityGracePeriodMs ?? 100
 
   function cleanupOldSessions(): void {
     const maxSessions = options.config.maxTrackedSessions
@@ -43,6 +48,10 @@ 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 {
@@ -51,11 +60,17 @@ export function createIdleNotificationScheduler(options: {
       clearTimeout(timer)
       pendingTimers.delete(sessionID)
     }
+    scheduledAt.delete(sessionID)
     sessionActivitySinceIdle.add(sessionID)
     notificationVersions.set(sessionID, (notificationVersions.get(sessionID) ?? 0) + 1)
   }
 
   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)
@@ -65,22 +80,26 @@ export function createIdleNotificationScheduler(options: {
   async function executeNotification(sessionID: string, version: number): Promise<void> {
     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)
@@ -126,6 +146,7 @@ export function createIdleNotificationScheduler(options: {
     if (executingNotifications.has(sessionID)) return
 
     sessionActivitySinceIdle.delete(sessionID)
+    scheduledAt.set(sessionID, Date.now())
 
     const currentVersion = (notificationVersions.get(sessionID) ?? 0) + 1
     notificationVersions.set(sessionID, currentVersion)
@@ -144,6 +165,7 @@ export function createIdleNotificationScheduler(options: {
     sessionActivitySinceIdle.delete(sessionID)
     notificationVersions.delete(sessionID)
     executingNotifications.delete(sessionID)
+    scheduledAt.delete(sessionID)
   }
 
   return {

src/hooks/session-notification.test.ts

@@ -195,8 +195,9 @@ describe("session-notification", () => {
     setMainSession(mainSessionID)
 
     const hook = createSessionNotification(createMockPluginInput(), {
-      idleConfirmationDelay: 100, // Long delay
+      idleConfirmationDelay: 100,
       skipIfIncompleteTodos: false,
+      activityGracePeriodMs: 0,
     })
 
     // when - session goes idle
@@ -272,6 +273,7 @@ describe("session-notification", () => {
     const hook = createSessionNotification(createMockPluginInput(), {
       idleConfirmationDelay: 50,
       skipIfIncompleteTodos: false,
+      activityGracePeriodMs: 0,
     })
 
     // when - session goes idle, then message.updated fires
@@ -306,6 +308,7 @@ describe("session-notification", () => {
     const hook = createSessionNotification(createMockPluginInput(), {
       idleConfirmationDelay: 50,
       skipIfIncompleteTodos: false,
+      activityGracePeriodMs: 0,
     })
 
     // when - session goes idle, then tool.execute.before fires
@@ -509,4 +512,75 @@ describe("session-notification", () => {
       }
     }
   })
+
+  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)
+  })
 })

src/hooks/session-notification.ts

@@ -24,6 +24,8 @@ interface SessionNotificationConfig {
   /** Maximum number of sessions to track before cleanup (default: 100) */
   maxTrackedSessions?: number
   enforceMainSessionFilter?: boolean
+  /** Grace period in ms to ignore late-arriving activity events after scheduling (default: 100) */
+  activityGracePeriodMs?: number
 }
 export function createSessionNotification(
   ctx: PluginInput,