fix(background-agent): keep stale-pruned tasks through notification cleanup
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
@@ -3422,7 +3422,7 @@ describe("BackgroundManager.pruneStaleTasksAndNotifications - removes pruned tas
|
|||||||
manager.shutdown()
|
manager.shutdown()
|
||||||
})
|
})
|
||||||
|
|
||||||
test("removes stale task from toast manager", () => {
|
test("removes stale task from toast manager", async () => {
|
||||||
//#given
|
//#given
|
||||||
const { removeTaskCalls, resetToastManager } = createToastRemoveTaskTracker()
|
const { removeTaskCalls, resetToastManager } = createToastRemoveTaskTracker()
|
||||||
const manager = createBackgroundManager()
|
const manager = createBackgroundManager()
|
||||||
@@ -3437,6 +3437,7 @@ describe("BackgroundManager.pruneStaleTasksAndNotifications - removes pruned tas
|
|||||||
|
|
||||||
//#when
|
//#when
|
||||||
pruneStaleTasksAndNotificationsForTest(manager)
|
pruneStaleTasksAndNotificationsForTest(manager)
|
||||||
|
await flushBackgroundNotifications()
|
||||||
|
|
||||||
//#then
|
//#then
|
||||||
expect(removeTaskCalls).toContain(staleTask.id)
|
expect(removeTaskCalls).toContain(staleTask.id)
|
||||||
@@ -3444,6 +3445,53 @@ describe("BackgroundManager.pruneStaleTasksAndNotifications - removes pruned tas
|
|||||||
manager.shutdown()
|
manager.shutdown()
|
||||||
resetToastManager()
|
resetToastManager()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("keeps stale task until notification cleanup after notifying parent", async () => {
|
||||||
|
//#given
|
||||||
|
const notifications: string[] = []
|
||||||
|
const { removeTaskCalls, resetToastManager } = createToastRemoveTaskTracker()
|
||||||
|
const client = {
|
||||||
|
session: {
|
||||||
|
prompt: async () => ({}),
|
||||||
|
promptAsync: async (args: { path: { id: string }; body: Record<string, unknown> & { noReply?: boolean; parts?: unknown[] } }) => {
|
||||||
|
const firstPart = args.body.parts?.[0]
|
||||||
|
if (firstPart && typeof firstPart === "object" && "text" in firstPart && typeof firstPart.text === "string") {
|
||||||
|
notifications.push(firstPart.text)
|
||||||
|
}
|
||||||
|
return {}
|
||||||
|
},
|
||||||
|
abort: async () => ({}),
|
||||||
|
messages: async () => ({ data: [] }),
|
||||||
|
},
|
||||||
|
}
|
||||||
|
const manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput)
|
||||||
|
const staleTask = createMockTask({
|
||||||
|
id: "task-stale-notify-cleanup",
|
||||||
|
sessionID: "session-stale-notify-cleanup",
|
||||||
|
parentSessionID: "parent-stale-notify-cleanup",
|
||||||
|
status: "running",
|
||||||
|
startedAt: new Date(Date.now() - 31 * 60 * 1000),
|
||||||
|
})
|
||||||
|
getTaskMap(manager).set(staleTask.id, staleTask)
|
||||||
|
getPendingByParent(manager).set(staleTask.parentSessionID, new Set([staleTask.id]))
|
||||||
|
|
||||||
|
//#when
|
||||||
|
pruneStaleTasksAndNotificationsForTest(manager)
|
||||||
|
await flushBackgroundNotifications()
|
||||||
|
|
||||||
|
//#then
|
||||||
|
const retainedTask = getTaskMap(manager).get(staleTask.id)
|
||||||
|
expect(retainedTask?.status).toBe("error")
|
||||||
|
expect(getTaskMap(manager).has(staleTask.id)).toBe(true)
|
||||||
|
expect(notifications).toHaveLength(1)
|
||||||
|
expect(notifications[0]).toContain("[ALL BACKGROUND TASKS COMPLETE]")
|
||||||
|
expect(notifications[0]).toContain(staleTask.description)
|
||||||
|
expect(getCompletionTimers(manager).has(staleTask.id)).toBe(true)
|
||||||
|
expect(removeTaskCalls).toContain(staleTask.id)
|
||||||
|
|
||||||
|
manager.shutdown()
|
||||||
|
resetToastManager()
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
describe("BackgroundManager.completionTimers - Memory Leak Fix", () => {
|
describe("BackgroundManager.completionTimers - Memory Leak Fix", () => {
|
||||||
|
|||||||
@@ -1382,8 +1382,13 @@ Use \`background_output(task_id="${task.id}")\` to retrieve this result when rea
|
|||||||
}
|
}
|
||||||
const timer = setTimeout(() => {
|
const timer = setTimeout(() => {
|
||||||
this.completionTimers.delete(taskId)
|
this.completionTimers.delete(taskId)
|
||||||
if (this.tasks.has(taskId)) {
|
const taskToRemove = this.tasks.get(taskId)
|
||||||
|
if (taskToRemove) {
|
||||||
this.clearNotificationsForTask(taskId)
|
this.clearNotificationsForTask(taskId)
|
||||||
|
if (taskToRemove.sessionID) {
|
||||||
|
subagentSessions.delete(taskToRemove.sessionID)
|
||||||
|
SessionCategoryRegistry.remove(taskToRemove.sessionID)
|
||||||
|
}
|
||||||
this.tasks.delete(taskId)
|
this.tasks.delete(taskId)
|
||||||
log("[background-agent] Removed completed task from memory:", taskId)
|
log("[background-agent] Removed completed task from memory:", taskId)
|
||||||
}
|
}
|
||||||
@@ -1418,11 +1423,21 @@ Use \`background_output(task_id="${task.id}")\` to retrieve this result when rea
|
|||||||
task.status = "error"
|
task.status = "error"
|
||||||
task.error = errorMessage
|
task.error = errorMessage
|
||||||
task.completedAt = new Date()
|
task.completedAt = new Date()
|
||||||
|
this.taskHistory.record(task.parentSessionID, { id: task.id, sessionID: task.sessionID, agent: task.agent, description: task.description, status: "error", category: task.category, startedAt: task.startedAt, completedAt: task.completedAt })
|
||||||
if (task.concurrencyKey) {
|
if (task.concurrencyKey) {
|
||||||
this.concurrencyManager.release(task.concurrencyKey)
|
this.concurrencyManager.release(task.concurrencyKey)
|
||||||
task.concurrencyKey = undefined
|
task.concurrencyKey = undefined
|
||||||
}
|
}
|
||||||
this.cleanupPendingByParent(task)
|
const existingTimer = this.completionTimers.get(taskId)
|
||||||
|
if (existingTimer) {
|
||||||
|
clearTimeout(existingTimer)
|
||||||
|
this.completionTimers.delete(taskId)
|
||||||
|
}
|
||||||
|
const idleTimer = this.idleDeferralTimers.get(taskId)
|
||||||
|
if (idleTimer) {
|
||||||
|
clearTimeout(idleTimer)
|
||||||
|
this.idleDeferralTimers.delete(taskId)
|
||||||
|
}
|
||||||
if (wasPending) {
|
if (wasPending) {
|
||||||
const key = task.model
|
const key = task.model
|
||||||
? `${task.model.providerID}/${task.model.modelID}`
|
? `${task.model.providerID}/${task.model.modelID}`
|
||||||
@@ -1438,16 +1453,10 @@ Use \`background_output(task_id="${task.id}")\` to retrieve this result when rea
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
this.clearNotificationsForTask(taskId)
|
this.markForNotification(task)
|
||||||
const toastManager = getTaskToastManager()
|
this.enqueueNotificationForParent(task.parentSessionID, () => this.notifyParentSession(task)).catch(err => {
|
||||||
if (toastManager) {
|
log("[background-agent] Error in notifyParentSession for stale-pruned task:", { taskId: task.id, error: err })
|
||||||
toastManager.removeTask(taskId)
|
})
|
||||||
}
|
|
||||||
this.tasks.delete(taskId)
|
|
||||||
if (task.sessionID) {
|
|
||||||
subagentSessions.delete(task.sessionID)
|
|
||||||
SessionCategoryRegistry.remove(task.sessionID)
|
|
||||||
}
|
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user