From 33d290b3464eb82dcffcf49c6cc56c34d9a5b197 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Mon, 16 Feb 2026 13:50:57 +0900 Subject: [PATCH] fix: add toast cleanup to all BackgroundManager task removal paths TaskToastManager entries were never removed when tasks completed via error, session deletion, stale pruning, or cancelled with skipNotification. Ghost entries accumulated indefinitely, causing the 'Queued (N)' count in toast messages to grow without bound. Added toastManager.removeTask() calls to all 4 missing cleanup paths: - session.error handler - session.deleted handler - cancelTask with skipNotification - pruneStaleTasksAndNotifications Closes #1866 --- src/features/background-agent/manager.test.ts | 133 ++++++++++++++++++ src/features/background-agent/manager.ts | 16 +++ 2 files changed, 149 insertions(+) diff --git a/src/features/background-agent/manager.test.ts b/src/features/background-agent/manager.test.ts index 35f1afe4..e926c870 100644 --- a/src/features/background-agent/manager.test.ts +++ b/src/features/background-agent/manager.test.ts @@ -6,6 +6,7 @@ import type { BackgroundTask, ResumeInput } from "./types" import { MIN_IDLE_TIME_MS } from "./constants" import { BackgroundManager } from "./manager" import { ConcurrencyManager } from "./concurrency" +import { initTaskToastManager, _resetTaskToastManagerForTesting } from "../task-toast-manager/manager" const TASK_TTL_MS = 30 * 60 * 1000 @@ -215,6 +216,23 @@ function stubNotifyParentSession(manager: BackgroundManager): void { ;(manager as unknown as { notifyParentSession: () => Promise }).notifyParentSession = async () => {} } +function createToastRemoveTaskTracker(): { removeTaskCalls: string[]; resetToastManager: () => void } { + _resetTaskToastManagerForTesting() + const toastManager = initTaskToastManager({ + tui: { showToast: async () => {} }, + } as unknown as PluginInput["client"]) + const removeTaskCalls: string[] = [] + const originalRemoveTask = toastManager.removeTask.bind(toastManager) + toastManager.removeTask = (taskId: string): void => { + removeTaskCalls.push(taskId) + originalRemoveTask(taskId) + } + return { + removeTaskCalls, + resetToastManager: _resetTaskToastManagerForTesting, + } +} + function getCleanupSignals(): Array { const signals: Array = ["SIGINT", "SIGTERM", "beforeExit", "exit"] if (process.platform === "win32") { @@ -1770,6 +1788,32 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => { const pendingSet = pendingByParent.get(task.parentSessionID) expect(pendingSet?.has(task.id) ?? false).toBe(false) }) + + test("should remove task from toast manager when notification is skipped", async () => { + //#given + const { removeTaskCalls, resetToastManager } = createToastRemoveTaskTracker() + const manager = createBackgroundManager() + const task = createMockTask({ + id: "task-cancel-skip-notification", + sessionID: "session-cancel-skip-notification", + parentSessionID: "parent-cancel-skip-notification", + status: "running", + }) + getTaskMap(manager).set(task.id, task) + + //#when + const cancelled = await manager.cancelTask(task.id, { + source: "test", + skipNotification: true, + }) + + //#then + expect(cancelled).toBe(true) + expect(removeTaskCalls).toContain(task.id) + + manager.shutdown() + resetToastManager() + }) }) describe("multiple keys process in parallel", () => { @@ -2730,6 +2774,43 @@ describe("BackgroundManager.handleEvent - session.deleted cascade", () => { manager.shutdown() }) + + test("should remove tasks from toast manager when session is deleted", () => { + //#given + const { removeTaskCalls, resetToastManager } = createToastRemoveTaskTracker() + const manager = createBackgroundManager() + const parentSessionID = "session-parent-toast" + const childTask = createMockTask({ + id: "task-child-toast", + sessionID: "session-child-toast", + parentSessionID, + status: "running", + }) + const grandchildTask = createMockTask({ + id: "task-grandchild-toast", + sessionID: "session-grandchild-toast", + parentSessionID: "session-child-toast", + status: "pending", + startedAt: undefined, + queuedAt: new Date(), + }) + const taskMap = getTaskMap(manager) + taskMap.set(childTask.id, childTask) + taskMap.set(grandchildTask.id, grandchildTask) + + //#when + manager.handleEvent({ + type: "session.deleted", + properties: { info: { id: parentSessionID } }, + }) + + //#then + expect(removeTaskCalls).toContain(childTask.id) + expect(removeTaskCalls).toContain(grandchildTask.id) + + manager.shutdown() + resetToastManager() + }) }) describe("BackgroundManager.handleEvent - session.error", () => { @@ -2777,6 +2858,35 @@ describe("BackgroundManager.handleEvent - session.error", () => { manager.shutdown() }) + test("removes errored task from toast manager", () => { + //#given + const { removeTaskCalls, resetToastManager } = createToastRemoveTaskTracker() + const manager = createBackgroundManager() + const sessionID = "ses_error_toast" + const task = createMockTask({ + id: "task-session-error-toast", + sessionID, + parentSessionID: "parent-session", + status: "running", + }) + getTaskMap(manager).set(task.id, task) + + //#when + manager.handleEvent({ + type: "session.error", + properties: { + sessionID, + error: { name: "UnknownError", message: "boom" }, + }, + }) + + //#then + expect(removeTaskCalls).toContain(task.id) + + manager.shutdown() + resetToastManager() + }) + test("ignores session.error for non-running tasks", () => { //#given const manager = createBackgroundManager() @@ -2922,6 +3032,29 @@ describe("BackgroundManager.pruneStaleTasksAndNotifications - removes pruned tas manager.shutdown() }) + + test("removes stale task from toast manager", () => { + //#given + const { removeTaskCalls, resetToastManager } = createToastRemoveTaskTracker() + const manager = createBackgroundManager() + const staleTask = createMockTask({ + id: "task-stale-toast", + sessionID: "session-stale-toast", + parentSessionID: "parent-session", + status: "running", + startedAt: new Date(Date.now() - 31 * 60 * 1000), + }) + getTaskMap(manager).set(staleTask.id, staleTask) + + //#when + pruneStaleTasksAndNotificationsForTest(manager) + + //#then + expect(removeTaskCalls).toContain(staleTask.id) + + manager.shutdown() + resetToastManager() + }) }) describe("BackgroundManager.completionTimers - Memory Leak Fix", () => { diff --git a/src/features/background-agent/manager.ts b/src/features/background-agent/manager.ts index 86ab03d3..a2eda592 100644 --- a/src/features/background-agent/manager.ts +++ b/src/features/background-agent/manager.ts @@ -783,6 +783,10 @@ export class BackgroundManager { this.cleanupPendingByParent(task) this.tasks.delete(task.id) this.clearNotificationsForTask(task.id) + const toastManager = getTaskToastManager() + if (toastManager) { + toastManager.removeTask(task.id) + } if (task.sessionID) { subagentSessions.delete(task.sessionID) } @@ -830,6 +834,10 @@ export class BackgroundManager { this.cleanupPendingByParent(task) this.tasks.delete(task.id) this.clearNotificationsForTask(task.id) + const toastManager = getTaskToastManager() + if (toastManager) { + toastManager.removeTask(task.id) + } if (task.sessionID) { subagentSessions.delete(task.sessionID) } @@ -1000,6 +1008,10 @@ export class BackgroundManager { } if (options?.skipNotification) { + const toastManager = getTaskToastManager() + if (toastManager) { + toastManager.removeTask(task.id) + } log(`[background-agent] Task cancelled via ${source} (notification skipped):`, task.id) return true } @@ -1413,6 +1425,10 @@ Use \`background_output(task_id="${task.id}")\` to retrieve this result when rea } } this.clearNotificationsForTask(taskId) + const toastManager = getTaskToastManager() + if (toastManager) { + toastManager.removeTask(taskId) + } this.tasks.delete(taskId) if (task.sessionID) { subagentSessions.delete(task.sessionID)