diff --git a/src/features/background-agent/manager.test.ts b/src/features/background-agent/manager.test.ts index 0c198f1c7..14a9cffcd 100644 --- a/src/features/background-agent/manager.test.ts +++ b/src/features/background-agent/manager.test.ts @@ -526,10 +526,16 @@ describe("BackgroundManager retry observability", () => { currentAttemptID: "att_retry_visibility", }) getTaskMap(manager).set(task.id, task) - const queuePendingNotification = mock(() => {}) + const queuePendingParentWake = mock(() => {}) ;(cast<{ - queuePendingNotification: (sessionId: string | undefined, notification: string) => void - }>(manager)).queuePendingNotification = queuePendingNotification + queuePendingParentWake: ( + sessionId: string, + notification: string, + promptContext: Record, + shouldReply: boolean, + delayMs?: number, + ) => void + }>(manager)).queuePendingParentWake = queuePendingParentWake //#when await (cast<{ @@ -540,9 +546,11 @@ describe("BackgroundManager retry observability", () => { }, "promptAsync.launch") //#then - expect(queuePendingNotification).toHaveBeenCalledTimes(1) - const [sessionID, notification] = queuePendingNotification.mock.calls[0] + expect(queuePendingParentWake).toHaveBeenCalledTimes(1) + const [sessionID, notification, promptContext, shouldReply] = queuePendingParentWake.mock.calls[0] expect(sessionID).toBe("parent-session") + expect(promptContext).toEqual({}) + expect(shouldReply).toBe(false) expect(notification).toContain("[BACKGROUND TASK RETRYING]") expect(notification).toContain("ses_retry_visibility") expect(notification).toContain("genai-proxy-openai/gpt-5.4-mini") @@ -551,7 +559,7 @@ describe("BackgroundManager retry observability", () => { test("queues a second parent-visible notification once the retry session ID is created", async () => { //#given - const queuePendingNotification = mock(() => {}) + const queuePendingParentWake = mock(() => {}) const client = { session: { get: async () => ({ data: { directory: tmpdir() } }), @@ -561,8 +569,14 @@ describe("BackgroundManager retry observability", () => { } const manager = new BackgroundManager({ pluginContext: createPluginInput(client) }) ;(cast<{ - queuePendingNotification: (sessionId: string | undefined, notification: string) => void - }>(manager)).queuePendingNotification = queuePendingNotification + queuePendingParentWake: ( + sessionId: string, + notification: string, + promptContext: Record, + shouldReply: boolean, + delayMs?: number, + ) => void + }>(manager)).queuePendingParentWake = queuePendingParentWake const task = createMockTask({ id: "bg_retry_ready", parentSessionId: "parent-session", @@ -623,7 +637,9 @@ describe("BackgroundManager retry observability", () => { }>(manager)).startTask(item) //#then - const notifications = cast>(queuePendingNotification.mock.calls).map((call) => call[1]) + const notifications = cast, boolean, number | undefined]>>( + queuePendingParentWake.mock.calls, + ).map((call) => call[1]) const retryReadyNotification = notifications.find((notification) => notification.includes("[BACKGROUND TASK RETRY SESSION READY]")) const expectedRetryLink = `http://127.0.0.1:4096/${Buffer.from(tmpdir()).toString("base64url")}/session/ses_retry_created` expect(retryReadyNotification).toBeDefined() @@ -637,7 +653,7 @@ describe("BackgroundManager retry observability", () => { test("builds retry-ready links from the parent session directory when it differs from the manager directory", async () => { //#given - const queuePendingNotification = mock(() => {}) + const queuePendingParentWake = mock(() => {}) const managerDirectory = "/manager/dir" const parentDirectory = "/parent/dir" const client = { @@ -649,8 +665,14 @@ describe("BackgroundManager retry observability", () => { } const manager = new BackgroundManager({ pluginContext: createPluginInput(client, managerDirectory) }) ;(cast<{ - queuePendingNotification: (sessionId: string | undefined, notification: string) => void - }>(manager)).queuePendingNotification = queuePendingNotification + queuePendingParentWake: ( + sessionId: string, + notification: string, + promptContext: Record, + shouldReply: boolean, + delayMs?: number, + ) => void + }>(manager)).queuePendingParentWake = queuePendingParentWake const task = createMockTask({ id: "bg_retry_ready_parent_dir", parentSessionId: "parent-session", @@ -699,9 +721,11 @@ describe("BackgroundManager retry observability", () => { }>(manager)).startTask({ task, input: taskInput, attemptID: "att_retry_ready_parent_dir" }) //#then - const retryReadyNotification = cast>(queuePendingNotification.mock.calls) - .map((call) => call[1]) - .find((notification) => notification.includes("[BACKGROUND TASK RETRY SESSION READY]")) + const retryReadyNotification = cast, boolean, number | undefined]>>( + queuePendingParentWake.mock.calls, + ) + .map((call) => call[1]) + .find((notification) => notification.includes("[BACKGROUND TASK RETRY SESSION READY]")) const expectedRetryLink = `http://127.0.0.1:4096/${Buffer.from(parentDirectory).toString("base64url")}/session/ses_retry_created_parent_dir` expect(retryReadyNotification).toBeDefined() expect(retryReadyNotification).toContain(expectedRetryLink) @@ -1566,10 +1590,10 @@ describe("BackgroundManager.notifyParentSession - aborted parent", () => { await waitForCoalescedFlush() //#then - const queuedNotifications = getPendingNotifications(manager).get("session-parent") ?? [] - expect(queuedNotifications).toHaveLength(1) - expect(queuedNotifications[0]).toContain("") - expect(queuedNotifications[0]).toContain("[ALL BACKGROUND TASKS COMPLETE]") + const pendingWake = getPendingParentWakes(manager).get("session-parent") + expect(pendingWake?.notifications).toHaveLength(1) + expect(pendingWake?.notifications[0]).toContain("") + expect(pendingWake?.notifications[0]).toContain("[ALL BACKGROUND TASKS COMPLETE]") manager.shutdown() }) @@ -1728,7 +1752,7 @@ describe("BackgroundManager.notifyParentSession - variant propagation", () => { }) describe("BackgroundManager.injectPendingNotificationsIntoChatMessage", () => { - test("should prepend queued notifications to first text part and clear queue", () => { + test("should defer queued notifications without mutating user text", () => { // given const manager = createBackgroundManager() manager.queuePendingNotification("session-parent", "queued-one") @@ -1741,9 +1765,11 @@ describe("BackgroundManager.injectPendingNotificationsIntoChatMessage", () => { manager.injectPendingNotificationsIntoChatMessage(output, "session-parent") // then - expect(output.parts[0].text).toContain("queued-one") - expect(output.parts[0].text).toContain("queued-two") - expect(output.parts[0].text).toContain("User prompt") + expect(output.parts).toEqual([{ type: "text", text: "User prompt" }]) + expect(getPendingParentWakes(manager).get("session-parent")?.notifications).toEqual([ + "queued-one\n\nqueued-two", + ]) + expect(getPendingParentWakes(manager).get("session-parent")?.shouldReply).toBe(false) expect(getPendingNotifications(manager).get("session-parent")).toBeUndefined() manager.shutdown() diff --git a/src/features/background-agent/manager.ts b/src/features/background-agent/manager.ts index d0b50677f..dd25cb212 100644 --- a/src/features/background-agent/manager.ts +++ b/src/features/background-agent/manager.ts @@ -818,7 +818,7 @@ export class BackgroundManager { ? `\n- Error: ${failedError}` : "" const retryModel = formatAttemptModelSummary(boundAttempt) ?? task.retryNotification.nextModel - this.queuePendingNotification( + this.queuePendingParentWake( task.parentSessionId, ` [BACKGROUND TASK RETRY SESSION READY] @@ -829,7 +829,10 @@ export class BackgroundManager { **Retry link:** ${retrySessionUrl}${failedSessionLine}${failedModelLine}${failedErrorLine}${retryModel ? `\n- Model: \`${retryModel}\`` : ""} The fallback retry session is now created and can be inspected directly. -` +`, + {}, + false, + PENDING_PARENT_WAKE_DEBOUNCE_MS, ) task.retryNotification = undefined } @@ -2069,7 +2072,7 @@ The fallback retry session is now created and can be inspected directly. const failedModelLine = failedModel ? `\n- Failed model: \`${failedModel}\`` : "" const failedErrorLine = previousAttempt?.error ? `\n- Error: ${previousAttempt.error}` : "" const nextModel = formatAttemptModelSummary(currentAttempt) - this.queuePendingNotification( + this.queuePendingParentWake( task.parentSessionId, ` [BACKGROUND TASK RETRYING] @@ -2077,7 +2080,10 @@ The fallback retry session is now created and can be inspected directly. **Description:** ${task.description}${sourceText}${failedSessionLine}${failedModelLine}${failedErrorLine}${nextModel ? `\n- Next model: \`${nextModel}\`` : ""} The task was re-queued on a fallback model after a retryable failure. -` +`, + {}, + false, + PENDING_PARENT_WAKE_DEBOUNCE_MS, ) }, }) @@ -2111,23 +2117,15 @@ The task was re-queued on a fallback model after a retryable failure. this.pendingNotifications.set(sessionID, existingNotifications) } - injectPendingNotificationsIntoChatMessage(output: { parts: Array<{ type: string; text?: string; [key: string]: unknown }> }, sessionID: string): void { + injectPendingNotificationsIntoChatMessage(_output: { parts: Array<{ type: string; text?: string; [key: string]: unknown }> }, sessionID: string): void { const pendingNotifications = this.pendingNotifications.get(sessionID) if (!pendingNotifications || pendingNotifications.length === 0) { return } - this.pendingNotifications.delete(sessionID) const notificationContent = pendingNotifications.join("\n\n") - const firstTextPartIndex = output.parts.findIndex((part) => part.type === "text") - - if (firstTextPartIndex === -1) { - output.parts.unshift(createInternalAgentTextPart(notificationContent)) - return - } - - const originalText = output.parts[firstTextPartIndex].text ?? "" - output.parts[firstTextPartIndex].text = `${notificationContent}\n\n---\n\n${originalText}` + this.pendingNotifications.delete(sessionID) + this.queuePendingParentWake(sessionID, notificationContent, {}, false, PENDING_PARENT_WAKE_DEBOUNCE_MS) } /** @@ -2748,7 +2746,15 @@ The task was re-queued on a fallback model after a retryable failure. log("[background-agent] Sent deferred parent wake:", { sessionID }) this.trackDispatchedParentWake(sessionID, latestWake) } catch (error) { - this.queuePendingNotification(sessionID, notificationContent) + const pendingWake = this.pendingParentWakes.get(sessionID) + if (pendingWake) { + pendingWake.notifications.unshift(...latestWake.notifications) + pendingWake.shouldReply = pendingWake.shouldReply || latestWake.shouldReply + pendingWake.promptContext = latestWake.promptContext + } else { + this.pendingParentWakes.set(sessionID, latestWake) + } + this.schedulePendingParentWakeFlush(sessionID) log("[background-agent] Failed to send deferred parent wake:", { sessionID, error }) } } diff --git a/src/hooks/background-notification/hook.ts b/src/hooks/background-notification/hook.ts index 0e31ba36f..d963fa4f6 100644 --- a/src/hooks/background-notification/hook.ts +++ b/src/hooks/background-notification/hook.ts @@ -28,12 +28,6 @@ const FORWARDED_EVENT_TYPES = new Set([ "session.status", ]) -/** - * Background notification hook - handles event routing to BackgroundManager. - * - * Notifications are now delivered directly via session.prompt({ noReply }) - * from the manager, so this hook only needs to handle event routing. - */ export function createBackgroundNotificationHook(manager: BackgroundManager) { const eventHandler = async ({ event }: EventInput) => { if (!FORWARDED_EVENT_TYPES.has(event.type)) return