From 07797e19759bf75f507e91a9df59530c31587559 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Tue, 12 May 2026 15:28:35 +0900 Subject: [PATCH] fix(background-agent): preserve direct all-complete replies --- src/features/background-agent/manager.ts | 178 ++---------------- .../task-completion-cleanup.test.ts | 50 +++-- 2 files changed, 42 insertions(+), 186 deletions(-) diff --git a/src/features/background-agent/manager.ts b/src/features/background-agent/manager.ts index efb812eb0..180c0a5fe 100644 --- a/src/features/background-agent/manager.ts +++ b/src/features/background-agent/manager.ts @@ -89,7 +89,6 @@ import { resolveSubagentSpawnContext, type SubagentSpawnContext, } from "./subagent-spawn-limits" -import { settleAfterSessionIdle } from "../../hooks/shared/session-idle-settle" type OpencodeClient = PluginInput["client"] @@ -100,15 +99,6 @@ type ParentWakePromptContext = { tools?: Record } -type PendingParentWake = { - promptContext: ParentWakePromptContext - notifications: string[] -} - -type SessionStatusInfo = { type?: string } - -const PENDING_PARENT_WAKE_RETRY_MS = 1_000 - interface MessagePartInfo { id?: string sessionID?: string @@ -230,8 +220,6 @@ export class BackgroundManager { private completedTaskSummaries: Map = new Map() private idleDeferralTimers: Map> = new Map() private notificationQueueByParent: Map> = new Map() - private pendingParentWakes: Map = new Map() - private pendingParentWakeTimers: Map> = new Map() private observedOutputSessions: Set = new Set() private observedIncompleteTodosBySession: Map = new Map() private rootDescendantCounts: Map @@ -1421,12 +1409,6 @@ The fallback retry session is now created and can be inspected directly. if (event.type === "session.idle") { if (!props || typeof props !== "object") return - const sessionID = resolveSessionEventID(props) - if (sessionID) { - void this.enqueueNotificationForParent(sessionID, () => this.flushPendingParentWake(sessionID)).catch((error) => { - log("[background-agent] Failed to flush pending parent wake:", { sessionID, error }) - }) - } handleSessionIdleBackgroundEvent({ properties: props as Record, findBySession: (id) => { @@ -2231,42 +2213,30 @@ The task was re-queued on a fallback model after a retryable failure. ...(variant !== undefined ? { variant } : {}), ...(resolvedTools ? { tools: resolvedTools } : {}), } - const shouldDeferReply = shouldReply && await this.isSessionActive(task.parentSessionId) - - if (shouldDeferReply) { - this.queuePendingParentWake(task.parentSessionId, notification, parentPromptContext) - log("[background-agent] Deferred notification until parent session is idle:", { + try { + await this.client.session.promptAsync({ + path: { id: task.parentSessionId }, + body: { + noReply: !shouldReply, + ...parentPromptContext, + parts: [createInternalAgentTextPart(notification)], + }, + }) + log("[background-agent] Sent notification to parent session:", { taskId: task.id, allComplete, isTaskFailure, + noReply: !shouldReply, }) - } else { - try { - await this.client.session.promptAsync({ - path: { id: task.parentSessionId }, - body: { - noReply: !shouldReply, - ...parentPromptContext, - parts: [createInternalAgentTextPart(notification)], - }, - }) - log("[background-agent] Sent notification to parent session:", { + } catch (error) { + if (isAbortedSessionError(error)) { + log("[background-agent] Parent session aborted while sending notification; continuing cleanup:", { taskId: task.id, - allComplete, - isTaskFailure, - noReply: !shouldReply, - deferredReply: false, + parentSessionID: task.parentSessionId, }) - } catch (error) { - if (isAbortedSessionError(error)) { - log("[background-agent] Parent session aborted while sending notification; continuing cleanup:", { - taskId: task.id, - parentSessionID: task.parentSessionId, - }) - this.queuePendingNotification(task.parentSessionId, notification) - } else { - log("[background-agent] Failed to send notification:", error) - } + this.queuePendingNotification(task.parentSessionId, notification) + } else { + log("[background-agent] Failed to send notification:", error) } } } else { @@ -2288,112 +2258,6 @@ The task was re-queued on a fallback model after a retryable failure. return false } - private async isSessionActive(sessionID: string): Promise { - const sessionStatusMethod = this.client?.session?.status - if (typeof sessionStatusMethod !== "function") { - return false - } - - try { - const statusResult = await this.client.session.status() - const statuses = normalizeSDKResponse( - statusResult, - {} as Record, - ) - const status = statuses[sessionID] - return typeof status?.type === "string" && isActiveSessionStatus(status.type) - } catch (error) { - log("[background-agent] Unable to check parent session status before wake:", { - sessionID, - error, - }) - return false - } - } - - private queuePendingParentWake( - sessionID: string, - notification: string, - promptContext: ParentWakePromptContext, - ): void { - const pendingWake = this.pendingParentWakes.get(sessionID) - if (pendingWake) { - pendingWake.notifications.push(notification) - pendingWake.promptContext = promptContext - } else { - this.pendingParentWakes.set(sessionID, { - promptContext, - notifications: [notification], - }) - } - this.schedulePendingParentWakeFlush(sessionID) - } - - private async flushPendingParentWake(sessionID: string): Promise { - const pendingWake = this.pendingParentWakes.get(sessionID) - if (!pendingWake) { - this.clearPendingParentWakeTimer(sessionID) - return - } - - if (await this.isSessionActive(sessionID)) { - this.schedulePendingParentWakeFlush(sessionID) - return - } - - this.pendingParentWakes.delete(sessionID) - this.clearPendingParentWakeTimer(sessionID) - await settleAfterSessionIdle() - - if (await this.isSessionActive(sessionID)) { - this.pendingParentWakes.set(sessionID, pendingWake) - this.schedulePendingParentWakeFlush(sessionID) - return - } - - const notificationContent = pendingWake.notifications.join("\n\n") - - try { - await this.client.session.promptAsync({ - path: { id: sessionID }, - body: { - noReply: false, - ...pendingWake.promptContext, - parts: [createInternalAgentTextPart(notificationContent)], - }, - }) - log("[background-agent] Sent deferred parent wake:", { sessionID }) - } catch (error) { - this.queuePendingNotification(sessionID, notificationContent) - log("[background-agent] Failed to send deferred parent wake:", { sessionID, error }) - } - } - - private schedulePendingParentWakeFlush(sessionID: string): void { - if (this.pendingParentWakeTimers.has(sessionID)) { - return - } - - const timer = setTimeout(() => { - this.pendingParentWakeTimers.delete(sessionID) - void this.enqueueNotificationForParent(sessionID, () => this.flushPendingParentWake(sessionID)).catch((error) => { - log("[background-agent] Failed to retry pending parent wake:", { sessionID, error }) - }) - }, PENDING_PARENT_WAKE_RETRY_MS) - - this.pendingParentWakeTimers.set(sessionID, timer) - } - - private clearPendingParentWakeTimer(sessionID: string): void { - const timer = this.pendingParentWakeTimers.get(sessionID) - if (!timer) { - return - } - - clearTimeout(timer) - this.pendingParentWakeTimers.delete(sessionID) - } - private pruneStaleTasksAndNotifications(allStatuses?: SessionStatusMap): void { pruneStaleTasksAndNotifications({ tasks: this.tasks, @@ -2707,11 +2571,6 @@ The task was re-queued on a fallback model after a retryable failure. } this.idleDeferralTimers.clear() - for (const timer of this.pendingParentWakeTimers.values()) { - clearTimeout(timer) - } - this.pendingParentWakeTimers.clear() - for (const sessionID of trackedSessionIDs) { subagentSessions.delete(sessionID) SessionCategoryRegistry.remove(sessionID) @@ -2724,7 +2583,6 @@ The task was re-queued on a fallback model after a retryable failure. this.pendingNotifications.clear() this.pendingByParent.clear() this.notificationQueueByParent.clear() - this.pendingParentWakes.clear() this.rootDescendantCounts.clear() this.queuesByKey.clear() this.processingKeys.clear() diff --git a/src/features/background-agent/task-completion-cleanup.test.ts b/src/features/background-agent/task-completion-cleanup.test.ts index 884ec31d6..0212a5e17 100644 --- a/src/features/background-agent/task-completion-cleanup.test.ts +++ b/src/features/background-agent/task-completion-cleanup.test.ts @@ -4,6 +4,7 @@ import type { PluginInput } from "@opencode-ai/plugin" import { TASK_CLEANUP_DELAY_MS } from "./constants" import { BackgroundManager } from "./manager" import type { BackgroundTask } from "./types" +import { OMO_INTERNAL_INITIATOR_MARKER } from "../../shared/internal-initiator-marker" type PromptAsyncCall = { path: { id: string } @@ -159,14 +160,6 @@ async function notifyParentSessionForTest(manager: BackgroundManager, task: Back return notifyParentSession.call(manager, task) } -function waitForDeferredWake(): Promise { - return new Promise((resolve) => setTimeout(resolve, 180)) -} - -function waitForDeferredWakeRetry(): Promise { - return new Promise((resolve) => setTimeout(resolve, 1_180)) -} - function getRequiredTimer(manager: BackgroundManager, taskID: string): ReturnType { const timer = getCompletionTimers(manager).get(taskID) expect(timer).toBeDefined() @@ -241,6 +234,7 @@ describe("BackgroundManager.notifyParentSession cleanup scheduling", () => { // then expect(promptAsyncCalls).toHaveLength(2) + expect(promptAsyncCalls[0]?.body.noReply).toBe(true) expect(getCompletionTimers(manager).size).toBe(2) const allCompleteCall = promptAsyncCalls[1] expect(allCompleteCall).toBeDefined() @@ -251,13 +245,14 @@ describe("BackgroundManager.notifyParentSession cleanup scheduling", () => { expect(allCompleteCall.body.noReply).toBe(false) const allCompletePayload = JSON.stringify(allCompleteCall.body.parts) expect(allCompletePayload).toContain("ALL BACKGROUND TASKS COMPLETE") + expect(allCompletePayload).toContain(OMO_INTERNAL_INITIATOR_MARKER) expect(allCompletePayload).toContain(taskA.id) expect(allCompletePayload).toContain(taskB.id) expect(allCompletePayload).toContain(taskA.description) expect(allCompletePayload).toContain(taskB.description) }) - test("#when parent session is busy #then all-complete notification does not start an overlapping parent reply", async () => { + test("#when parent session is busy #then all-complete notification keeps the direct 4.0.0 parent prompt behavior", async () => { // given const sessionStatuses: Record = { "parent-1": { type: "busy" }, @@ -272,10 +267,14 @@ describe("BackgroundManager.notifyParentSession cleanup scheduling", () => { await notifyParentSessionForTest(manager, task) // then - expect(promptAsyncCalls).toHaveLength(0) + expect(promptAsyncCalls).toHaveLength(1) + expect(promptAsyncCalls[0]?.body.noReply).toBe(false) + const notificationPayload = JSON.stringify(promptAsyncCalls[0]?.body.parts) + expect(notificationPayload).toContain("ALL BACKGROUND TASKS COMPLETE") + expect(notificationPayload).toContain(OMO_INTERNAL_INITIATOR_MARKER) }) - test("#when deferred parent session becomes idle #then completion notification wakes the parent without a pointer reminder", async () => { + test("#when busy parent later becomes idle #then completion notification is not replayed as a second parent prompt", async () => { // given const sessionStatuses: Record = { "parent-1": { type: "busy" }, @@ -286,21 +285,22 @@ describe("BackgroundManager.notifyParentSession cleanup scheduling", () => { getTasks(manager).set(task.id, task) getPendingByParent(manager).set(task.parentSessionId, new Set([task.id])) await notifyParentSessionForTest(manager, task) + expect(promptAsyncCalls).toHaveLength(1) // when sessionStatuses["parent-1"] = { type: "idle" } manager.handleEvent({ type: "session.idle", properties: { sessionID: "parent-1" } }) - await waitForDeferredWake() + await Promise.resolve() // then expect(promptAsyncCalls).toHaveLength(1) expect(promptAsyncCalls[0]?.body.noReply).toBe(false) - const wakePayload = JSON.stringify(promptAsyncCalls[0]?.body.parts) - expect(wakePayload).toContain("ALL BACKGROUND TASKS COMPLETE") - expect(wakePayload).not.toContain("BACKGROUND TASK NOTIFICATION READY") + const notificationPayload = JSON.stringify(promptAsyncCalls[0]?.body.parts) + expect(notificationPayload).toContain("ALL BACKGROUND TASKS COMPLETE") + expect(notificationPayload).not.toContain("BACKGROUND TASK NOTIFICATION READY") }) - test("#when a single background task finishes during a stale busy parent status #then completion notification is retried after the parent becomes idle", async () => { + test("#when a single background task finishes during a stale busy parent status #then no deferred wake is scheduled", async () => { // given const sessionStatuses: Record = { "parent-1": { type: "busy" }, @@ -314,22 +314,23 @@ describe("BackgroundManager.notifyParentSession cleanup scheduling", () => { // when await notifyParentSessionForTest(manager, task) sessionStatuses["parent-1"] = { type: "idle" } - await waitForDeferredWakeRetry() + await new Promise((resolve) => setTimeout(resolve, 1_180)) // then expect(promptAsyncCalls).toHaveLength(1) expect(promptAsyncCalls[0]?.body.noReply).toBe(false) - const wakePayload = JSON.stringify(promptAsyncCalls[0]?.body.parts) - expect(wakePayload).toContain("ALL BACKGROUND TASKS COMPLETE") - expect(wakePayload).not.toContain("BACKGROUND TASK NOTIFICATION READY") + const notificationPayload = JSON.stringify(promptAsyncCalls[0]?.body.parts) + expect(notificationPayload).toContain("ALL BACKGROUND TASKS COMPLETE") + expect(notificationPayload).not.toContain("BACKGROUND TASK NOTIFICATION READY") }) - test("#when deferred completion notification send fails #then notification is queued for the next user message", async () => { + test("#when completion notification send is aborted #then notification is queued for the next user message", async () => { // given const sessionStatuses: Record = { "parent-1": { type: "busy" }, } - const promptError = new Error("promptAsync failed") + const promptError = new Error("Request aborted while waiting for input") + promptError.name = "MessageAbortedError" const { manager, promptAsyncCalls } = createManager(true, sessionStatuses, async () => { throw promptError }) @@ -337,12 +338,9 @@ describe("BackgroundManager.notifyParentSession cleanup scheduling", () => { const task = createTask({ id: "task-a", parentSessionId: "parent-1", description: "task A", status: "completed", completedAt: new Date("2026-03-11T00:01:00.000Z") }) getTasks(manager).set(task.id, task) getPendingByParent(manager).set(task.parentSessionId, new Set([task.id])) - await notifyParentSessionForTest(manager, task) // when - sessionStatuses["parent-1"] = { type: "idle" } - manager.handleEvent({ type: "session.idle", properties: { sessionID: "parent-1" } }) - await waitForDeferredWake() + await notifyParentSessionForTest(manager, task) // then expect(promptAsyncCalls).toHaveLength(1)