Merge pull request #1608 from code-yeongyu/fix/114-cascade-cancel
fix: cascade cancel descendant tasks when parent session is deleted (#114)
This commit is contained in:
@@ -2284,6 +2284,69 @@ describe("BackgroundManager.shutdown session abort", () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe("BackgroundManager.handleEvent - session.deleted cascade", () => {
|
||||
test("should cancel descendant tasks when parent session is deleted", () => {
|
||||
// given
|
||||
const manager = createBackgroundManager()
|
||||
const parentSessionID = "session-parent"
|
||||
const childTask = createMockTask({
|
||||
id: "task-child",
|
||||
sessionID: "session-child",
|
||||
parentSessionID,
|
||||
status: "running",
|
||||
})
|
||||
const siblingTask = createMockTask({
|
||||
id: "task-sibling",
|
||||
sessionID: "session-sibling",
|
||||
parentSessionID,
|
||||
status: "running",
|
||||
})
|
||||
const grandchildTask = createMockTask({
|
||||
id: "task-grandchild",
|
||||
sessionID: "session-grandchild",
|
||||
parentSessionID: "session-child",
|
||||
status: "pending",
|
||||
startedAt: undefined,
|
||||
queuedAt: new Date(),
|
||||
})
|
||||
const unrelatedTask = createMockTask({
|
||||
id: "task-unrelated",
|
||||
sessionID: "session-unrelated",
|
||||
parentSessionID: "other-parent",
|
||||
status: "running",
|
||||
})
|
||||
|
||||
const taskMap = getTaskMap(manager)
|
||||
taskMap.set(childTask.id, childTask)
|
||||
taskMap.set(siblingTask.id, siblingTask)
|
||||
taskMap.set(grandchildTask.id, grandchildTask)
|
||||
taskMap.set(unrelatedTask.id, unrelatedTask)
|
||||
|
||||
const pendingByParent = getPendingByParent(manager)
|
||||
pendingByParent.set(parentSessionID, new Set([childTask.id, siblingTask.id]))
|
||||
pendingByParent.set("session-child", new Set([grandchildTask.id]))
|
||||
|
||||
// when
|
||||
manager.handleEvent({
|
||||
type: "session.deleted",
|
||||
properties: { info: { id: parentSessionID } },
|
||||
})
|
||||
|
||||
// then
|
||||
expect(taskMap.has(childTask.id)).toBe(false)
|
||||
expect(taskMap.has(siblingTask.id)).toBe(false)
|
||||
expect(taskMap.has(grandchildTask.id)).toBe(false)
|
||||
expect(taskMap.has(unrelatedTask.id)).toBe(true)
|
||||
expect(childTask.status).toBe("cancelled")
|
||||
expect(siblingTask.status).toBe("cancelled")
|
||||
expect(grandchildTask.status).toBe("cancelled")
|
||||
expect(pendingByParent.get(parentSessionID)).toBeUndefined()
|
||||
expect(pendingByParent.get("session-child")).toBeUndefined()
|
||||
|
||||
manager.shutdown()
|
||||
})
|
||||
})
|
||||
|
||||
describe("BackgroundManager.completionTimers - Memory Leak Fix", () => {
|
||||
function getCompletionTimers(manager: BackgroundManager): Map<string, ReturnType<typeof setTimeout>> {
|
||||
return (manager as unknown as { completionTimers: Map<string, ReturnType<typeof setTimeout>> }).completionTimers
|
||||
|
||||
@@ -736,34 +736,47 @@ export class BackgroundManager {
|
||||
if (!info || typeof info.id !== "string") return
|
||||
const sessionID = info.id
|
||||
|
||||
const task = this.findBySession(sessionID)
|
||||
if (!task) return
|
||||
|
||||
if (task.status === "running") {
|
||||
task.status = "cancelled"
|
||||
task.completedAt = new Date()
|
||||
task.error = "Session deleted"
|
||||
const tasksToCancel = new Map<string, BackgroundTask>()
|
||||
const directTask = this.findBySession(sessionID)
|
||||
if (directTask) {
|
||||
tasksToCancel.set(directTask.id, directTask)
|
||||
}
|
||||
for (const descendant of this.getAllDescendantTasks(sessionID)) {
|
||||
tasksToCancel.set(descendant.id, descendant)
|
||||
}
|
||||
|
||||
if (task.concurrencyKey) {
|
||||
this.concurrencyManager.release(task.concurrencyKey)
|
||||
task.concurrencyKey = undefined
|
||||
}
|
||||
const existingTimer = this.completionTimers.get(task.id)
|
||||
if (existingTimer) {
|
||||
clearTimeout(existingTimer)
|
||||
this.completionTimers.delete(task.id)
|
||||
}
|
||||
if (tasksToCancel.size === 0) return
|
||||
|
||||
const idleTimer = this.idleDeferralTimers.get(task.id)
|
||||
if (idleTimer) {
|
||||
clearTimeout(idleTimer)
|
||||
this.idleDeferralTimers.delete(task.id)
|
||||
for (const task of tasksToCancel.values()) {
|
||||
if (task.status === "running" || task.status === "pending") {
|
||||
void this.cancelTask(task.id, {
|
||||
source: "session.deleted",
|
||||
reason: "Session deleted",
|
||||
skipNotification: true,
|
||||
}).catch(err => {
|
||||
log("[background-agent] Failed to cancel task on session.deleted:", { taskId: task.id, error: err })
|
||||
})
|
||||
}
|
||||
|
||||
const existingTimer = this.completionTimers.get(task.id)
|
||||
if (existingTimer) {
|
||||
clearTimeout(existingTimer)
|
||||
this.completionTimers.delete(task.id)
|
||||
}
|
||||
|
||||
const idleTimer = this.idleDeferralTimers.get(task.id)
|
||||
if (idleTimer) {
|
||||
clearTimeout(idleTimer)
|
||||
this.idleDeferralTimers.delete(task.id)
|
||||
}
|
||||
|
||||
this.cleanupPendingByParent(task)
|
||||
this.tasks.delete(task.id)
|
||||
this.clearNotificationsForTask(task.id)
|
||||
if (task.sessionID) {
|
||||
subagentSessions.delete(task.sessionID)
|
||||
}
|
||||
}
|
||||
this.cleanupPendingByParent(task)
|
||||
this.tasks.delete(task.id)
|
||||
this.clearNotificationsForTask(task.id)
|
||||
subagentSessions.delete(sessionID)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user