luna/oh-my-opencode
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(background-agent): preserve terminal tasks until notification cleanup

Browse Source 
Route terminal task cleanup through parent notifications so cancelled and errored tasks stay visible until delayed cleanup finishes.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
YeonGyu-Kim
2026-03-11 18:20:20 +09:00
parent 8c5f9b8082
commit e3b17da4bd
2 changed files with 145 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/features/background-agent/manager.test.ts
+93 -16
View File
@@ -224,6 +224,12 @@ function stubNotifyParentSession(manager: BackgroundManager): void {
;(manager as unknown as { notifyParentSession: () => Promise<void> }).notifyParentSession = async () => {}
}
async function flushBackgroundNotifications(): Promise<void> {
for (let i = 0; i < 6; i++) {
await Promise.resolve()
}
}
function createToastRemoveTaskTracker(): { removeTaskCalls: string[]; resetToastManager: () => void } {
_resetTaskToastManagerForTesting()
const toastManager = initTaskToastManager({
@@ -1306,11 +1312,20 @@ describe("BackgroundManager.tryCompleteTask", () => {
expect(abortedSessionIDs).toEqual(["session-1"])
})
test("should clean pendingByParent even when notifyParentSession throws", async () => {
test("should clean pendingByParent even when promptAsync notification fails", async () => {
// given
;(manager as unknown as { notifyParentSession: () => Promise<void> }).notifyParentSession = async () => {
throw new Error("notify failed")
const client = {
session: {
prompt: async () => ({}),
promptAsync: async () => {
throw new Error("notify failed")
},
abort: async () => ({}),
messages: async () => ({ data: [] }),
},
}
manager.shutdown()
manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput)
const task: BackgroundTask = {
id: "task-pending-cleanup",
@@ -1518,7 +1533,7 @@ describe("BackgroundManager.tryCompleteTask", () => {
// then
expect(rejectedCount).toBe(0)
expect(promptBodies.length).toBe(2)
expect(promptBodies.some((b) => b.noReply === false)).toBe(true)
expect(promptBodies.filter((body) => body.noReply === false)).toHaveLength(1)
})
})
@@ -2026,7 +2041,6 @@ describe("BackgroundManager - Non-blocking Queue Integration", () => {
test("should cancel running task and release concurrency", async () => {
// given
const manager = createBackgroundManager()
stubNotifyParentSession(manager)
const concurrencyManager = getConcurrencyManager(manager)
const concurrencyKey = "test-provider/test-model"
@@ -2984,7 +2998,7 @@ describe("BackgroundManager.shutdown session abort", () => {
})
describe("BackgroundManager.handleEvent - session.deleted cascade", () => {
test("should cancel descendant tasks when parent session is deleted", () => {
test("should cancel descendant tasks and keep them until delayed cleanup", async () => {
// given
const manager = createBackgroundManager()
const parentSessionID = "session-parent"
@@ -3031,21 +3045,26 @@ describe("BackgroundManager.handleEvent - session.deleted cascade", () => {
properties: { info: { id: parentSessionID } },
})
await flushBackgroundNotifications()
// 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(childTask.id)).toBe(true)
expect(taskMap.has(siblingTask.id)).toBe(true)
expect(taskMap.has(grandchildTask.id)).toBe(true)
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()
expect(getCompletionTimers(manager).has(childTask.id)).toBe(true)
expect(getCompletionTimers(manager).has(siblingTask.id)).toBe(true)
expect(getCompletionTimers(manager).has(grandchildTask.id)).toBe(true)
manager.shutdown()
})
test("should remove tasks from toast manager when session is deleted", () => {
test("should remove cancelled tasks from toast manager while preserving delayed cleanup", async () => {
//#given
const { removeTaskCalls, resetToastManager } = createToastRemoveTaskTracker()
const manager = createBackgroundManager()
@@ -3074,9 +3093,13 @@ describe("BackgroundManager.handleEvent - session.deleted cascade", () => {
properties: { info: { id: parentSessionID } },
})
await flushBackgroundNotifications()
//#then
expect(removeTaskCalls).toContain(childTask.id)
expect(removeTaskCalls).toContain(grandchildTask.id)
expect(getCompletionTimers(manager).has(childTask.id)).toBe(true)
expect(getCompletionTimers(manager).has(grandchildTask.id)).toBe(true)
manager.shutdown()
resetToastManager()
@@ -3139,7 +3162,7 @@ describe("BackgroundManager.handleEvent - session.error", () => {
return task
}
test("sets task to error, releases concurrency, and cleans up", async () => {
test("sets task to error, releases concurrency, and keeps it until delayed cleanup", async () => {
//#given
const manager = createBackgroundManager()
const concurrencyManager = getConcurrencyManager(manager)
@@ -3172,18 +3195,21 @@ describe("BackgroundManager.handleEvent - session.error", () => {
},
})
await flushBackgroundNotifications()
//#then
expect(task.status).toBe("error")
expect(task.error).toBe("Model not found: kimi-for-coding/k2p5.")
expect(task.completedAt).toBeInstanceOf(Date)
expect(concurrencyManager.getCount(concurrencyKey)).toBe(0)
expect(getTaskMap(manager).has(task.id)).toBe(false)
expect(getTaskMap(manager).has(task.id)).toBe(true)
expect(getPendingByParent(manager).get(task.parentSessionID)).toBeUndefined()
expect(getCompletionTimers(manager).has(task.id)).toBe(true)
manager.shutdown()
})
test("removes errored task from toast manager", () => {
test("should remove errored task from toast manager while preserving delayed cleanup", async () => {
//#given
const { removeTaskCalls, resetToastManager } = createToastRemoveTaskTracker()
const manager = createBackgroundManager()
@@ -3205,8 +3231,11 @@ describe("BackgroundManager.handleEvent - session.error", () => {
},
})
await flushBackgroundNotifications()
//#then
expect(removeTaskCalls).toContain(task.id)
expect(getCompletionTimers(manager).has(task.id)).toBe(true)
manager.shutdown()
resetToastManager()
@@ -3489,7 +3518,7 @@ describe("BackgroundManager.pruneStaleTasksAndNotifications - removes pruned tas
manager.shutdown()
})
test("removes stale task from toast manager", () => {
test("removes stale task from toast manager", async () => {
//#given
const { removeTaskCalls, resetToastManager } = createToastRemoveTaskTracker()
const manager = createBackgroundManager()
@@ -3504,6 +3533,7 @@ describe("BackgroundManager.pruneStaleTasksAndNotifications - removes pruned tas
//#when
pruneStaleTasksAndNotificationsForTest(manager)
await flushBackgroundNotifications()
//#then
expect(removeTaskCalls).toContain(staleTask.id)
@@ -3511,6 +3541,53 @@ describe("BackgroundManager.pruneStaleTasksAndNotifications - removes pruned tas
manager.shutdown()
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", () => {
@@ -3614,7 +3691,7 @@ describe("BackgroundManager.completionTimers - Memory Leak Fix", () => {
expect(completionTimers.size).toBe(0)
})
test("should cancel timer when task is deleted via session.deleted", () => {
test("should preserve cleanup timer when terminal task session is deleted", () => {
// given
const manager = createBackgroundManager()
const task: BackgroundTask = {
@@ -3643,7 +3720,7 @@ describe("BackgroundManager.completionTimers - Memory Leak Fix", () => {
})
// then
expect(completionTimers.has(task.id)).toBe(false)
expect(completionTimers.has(task.id)).toBe(true)
manager.shutdown()
})