Merge pull request #1874 from code-yeongyu/fix/toast-manager-ghost-entries
fix: add toast cleanup to all BackgroundManager task removal paths
This commit is contained in:
commit
dd91a7d990
@ -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<void> }).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<NodeJS.Signals | "beforeExit" | "exit"> {
|
||||
const signals: Array<NodeJS.Signals | "beforeExit" | "exit"> = ["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", () => {
|
||||
|
||||
@ -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)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user