From d3574a392f61b712389a05d4d2a46697df55fdd8 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Mon, 16 Feb 2026 15:56:40 +0900 Subject: [PATCH] fix: cancel completion timer on resume and prevent silent notification drop --- src/features/background-agent/manager.test.ts | 102 ++++++++++++++++-- src/features/background-agent/manager.ts | 15 ++- 2 files changed, 106 insertions(+), 11 deletions(-) diff --git a/src/features/background-agent/manager.test.ts b/src/features/background-agent/manager.test.ts index e926c8709..38ade254a 100644 --- a/src/features/background-agent/manager.test.ts +++ b/src/features/background-agent/manager.test.ts @@ -191,6 +191,10 @@ function getPendingByParent(manager: BackgroundManager): Map return (manager as unknown as { pendingByParent: Map> }).pendingByParent } +function getCompletionTimers(manager: BackgroundManager): Map> { + return (manager as unknown as { completionTimers: Map> }).completionTimers +} + function getQueuesByKey( manager: BackgroundManager ): Map> { @@ -912,7 +916,7 @@ describe("BackgroundManager.notifyParentSession - dynamic message lookup", () => }) describe("BackgroundManager.notifyParentSession - aborted parent", () => { - test("should skip notification when parent session is aborted", async () => { + test("should fall back and still notify when parent session messages are aborted", async () => { //#given let promptCalled = false const promptMock = async () => { @@ -951,7 +955,7 @@ describe("BackgroundManager.notifyParentSession - aborted parent", () => { .notifyParentSession(task) //#then - expect(promptCalled).toBe(false) + expect(promptCalled).toBe(true) manager.shutdown() }) @@ -3058,10 +3062,6 @@ describe("BackgroundManager.pruneStaleTasksAndNotifications - removes pruned tas }) describe("BackgroundManager.completionTimers - Memory Leak Fix", () => { - function getCompletionTimers(manager: BackgroundManager): Map> { - return (manager as unknown as { completionTimers: Map> }).completionTimers - } - function setCompletionTimer(manager: BackgroundManager, taskId: string): void { const completionTimers = getCompletionTimers(manager) const timer = setTimeout(() => { @@ -3587,3 +3587,93 @@ describe("BackgroundManager.handleEvent - non-tool event lastUpdate", () => { expect(task.status).toBe("running") }) }) + +describe("BackgroundManager regression fixes - resume and aborted notification", () => { + test("should keep resumed task in memory after previous completion timer deadline", async () => { + //#given + const client = { + session: { + prompt: async () => ({}), + promptAsync: async () => ({}), + abort: async () => ({}), + }, + } + const manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput) + + const task: BackgroundTask = { + id: "task-resume-timer-regression", + sessionID: "session-resume-timer-regression", + parentSessionID: "parent-session", + parentMessageID: "msg-1", + description: "resume timer regression", + prompt: "test", + agent: "explore", + status: "completed", + startedAt: new Date(), + completedAt: new Date(), + concurrencyGroup: "explore", + } + getTaskMap(manager).set(task.id, task) + + const completionTimers = getCompletionTimers(manager) + const timer = setTimeout(() => { + completionTimers.delete(task.id) + getTaskMap(manager).delete(task.id) + }, 25) + completionTimers.set(task.id, timer) + + //#when + await manager.resume({ + sessionId: "session-resume-timer-regression", + prompt: "resume task", + parentSessionID: "parent-session-2", + parentMessageID: "msg-2", + }) + await new Promise((resolve) => setTimeout(resolve, 60)) + + //#then + expect(getTaskMap(manager).has(task.id)).toBe(true) + expect(completionTimers.has(task.id)).toBe(false) + + manager.shutdown() + }) + + test("should start cleanup timer even when promptAsync aborts", async () => { + //#given + const client = { + session: { + prompt: async () => ({}), + promptAsync: async () => { + const error = new Error("User aborted") + error.name = "MessageAbortedError" + throw error + }, + abort: async () => ({}), + messages: async () => ({ data: [] }), + }, + } + const manager = new BackgroundManager({ client, directory: tmpdir() } as unknown as PluginInput) + const task: BackgroundTask = { + id: "task-aborted-cleanup-regression", + sessionID: "session-aborted-cleanup-regression", + parentSessionID: "parent-session", + parentMessageID: "msg-1", + description: "aborted prompt cleanup regression", + prompt: "test", + agent: "explore", + status: "completed", + startedAt: new Date(), + completedAt: new Date(), + } + getTaskMap(manager).set(task.id, task) + getPendingByParent(manager).set(task.parentSessionID, new Set([task.id])) + + //#when + await (manager as unknown as { notifyParentSession: (task: BackgroundTask) => Promise }).notifyParentSession(task) + + //#then + expect(getCompletionTimers(manager).has(task.id)).toBe(true) + + manager.shutdown() + }) +}) diff --git a/src/features/background-agent/manager.ts b/src/features/background-agent/manager.ts index a2eda592d..7baca91e4 100644 --- a/src/features/background-agent/manager.ts +++ b/src/features/background-agent/manager.ts @@ -528,6 +528,12 @@ export class BackgroundManager { return existingTask } + const completionTimer = this.completionTimers.get(existingTask.id) + if (completionTimer) { + clearTimeout(completionTimer) + this.completionTimers.delete(existingTask.id) + } + // Re-acquire concurrency using the persisted concurrency group const concurrencyKey = existingTask.concurrencyGroup ?? existingTask.agent await this.concurrencyManager.acquire(concurrencyKey) @@ -1251,11 +1257,10 @@ Use \`background_output(task_id="${task.id}")\` to retrieve this result when rea } } catch (error) { if (this.isAbortedSessionError(error)) { - log("[background-agent] Parent session aborted, skipping notification:", { + log("[background-agent] Parent session aborted while loading messages; using messageDir fallback:", { taskId: task.id, parentSessionID: task.parentSessionID, }) - return } const messageDir = getMessageDir(task.parentSessionID) const currentMessage = messageDir ? findNearestMessageWithFields(messageDir) : null @@ -1289,13 +1294,13 @@ Use \`background_output(task_id="${task.id}")\` to retrieve this result when rea }) } catch (error) { if (this.isAbortedSessionError(error)) { - log("[background-agent] Parent session aborted, skipping notification:", { + log("[background-agent] Parent session aborted while sending notification; continuing cleanup:", { taskId: task.id, parentSessionID: task.parentSessionID, }) - return + } else { + log("[background-agent] Failed to send notification:", error) } - log("[background-agent] Failed to send notification:", error) } if (allComplete) {