fix(background-agent): defer retry notifications
This commit is contained in:
@@ -526,10 +526,16 @@ describe("BackgroundManager retry observability", () => {
|
|||||||
currentAttemptID: "att_retry_visibility",
|
currentAttemptID: "att_retry_visibility",
|
||||||
})
|
})
|
||||||
getTaskMap(manager).set(task.id, task)
|
getTaskMap(manager).set(task.id, task)
|
||||||
const queuePendingNotification = mock(() => {})
|
const queuePendingParentWake = mock(() => {})
|
||||||
;(cast<{
|
;(cast<{
|
||||||
queuePendingNotification: (sessionId: string | undefined, notification: string) => void
|
queuePendingParentWake: (
|
||||||
}>(manager)).queuePendingNotification = queuePendingNotification
|
sessionId: string,
|
||||||
|
notification: string,
|
||||||
|
promptContext: Record<string, unknown>,
|
||||||
|
shouldReply: boolean,
|
||||||
|
delayMs?: number,
|
||||||
|
) => void
|
||||||
|
}>(manager)).queuePendingParentWake = queuePendingParentWake
|
||||||
|
|
||||||
//#when
|
//#when
|
||||||
await (cast<{
|
await (cast<{
|
||||||
@@ -540,9 +546,11 @@ describe("BackgroundManager retry observability", () => {
|
|||||||
}, "promptAsync.launch")
|
}, "promptAsync.launch")
|
||||||
|
|
||||||
//#then
|
//#then
|
||||||
expect(queuePendingNotification).toHaveBeenCalledTimes(1)
|
expect(queuePendingParentWake).toHaveBeenCalledTimes(1)
|
||||||
const [sessionID, notification] = queuePendingNotification.mock.calls[0]
|
const [sessionID, notification, promptContext, shouldReply] = queuePendingParentWake.mock.calls[0]
|
||||||
expect(sessionID).toBe("parent-session")
|
expect(sessionID).toBe("parent-session")
|
||||||
|
expect(promptContext).toEqual({})
|
||||||
|
expect(shouldReply).toBe(false)
|
||||||
expect(notification).toContain("[BACKGROUND TASK RETRYING]")
|
expect(notification).toContain("[BACKGROUND TASK RETRYING]")
|
||||||
expect(notification).toContain("ses_retry_visibility")
|
expect(notification).toContain("ses_retry_visibility")
|
||||||
expect(notification).toContain("genai-proxy-openai/gpt-5.4-mini")
|
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 () => {
|
test("queues a second parent-visible notification once the retry session ID is created", async () => {
|
||||||
//#given
|
//#given
|
||||||
const queuePendingNotification = mock(() => {})
|
const queuePendingParentWake = mock(() => {})
|
||||||
const client = {
|
const client = {
|
||||||
session: {
|
session: {
|
||||||
get: async () => ({ data: { directory: tmpdir() } }),
|
get: async () => ({ data: { directory: tmpdir() } }),
|
||||||
@@ -561,8 +569,14 @@ describe("BackgroundManager retry observability", () => {
|
|||||||
}
|
}
|
||||||
const manager = new BackgroundManager({ pluginContext: createPluginInput(client) })
|
const manager = new BackgroundManager({ pluginContext: createPluginInput(client) })
|
||||||
;(cast<{
|
;(cast<{
|
||||||
queuePendingNotification: (sessionId: string | undefined, notification: string) => void
|
queuePendingParentWake: (
|
||||||
}>(manager)).queuePendingNotification = queuePendingNotification
|
sessionId: string,
|
||||||
|
notification: string,
|
||||||
|
promptContext: Record<string, unknown>,
|
||||||
|
shouldReply: boolean,
|
||||||
|
delayMs?: number,
|
||||||
|
) => void
|
||||||
|
}>(manager)).queuePendingParentWake = queuePendingParentWake
|
||||||
const task = createMockTask({
|
const task = createMockTask({
|
||||||
id: "bg_retry_ready",
|
id: "bg_retry_ready",
|
||||||
parentSessionId: "parent-session",
|
parentSessionId: "parent-session",
|
||||||
@@ -623,7 +637,9 @@ describe("BackgroundManager retry observability", () => {
|
|||||||
}>(manager)).startTask(item)
|
}>(manager)).startTask(item)
|
||||||
|
|
||||||
//#then
|
//#then
|
||||||
const notifications = cast<Array<[string | undefined, string]>>(queuePendingNotification.mock.calls).map((call) => call[1])
|
const notifications = cast<Array<[string, string, Record<string, unknown>, boolean, number | undefined]>>(
|
||||||
|
queuePendingParentWake.mock.calls,
|
||||||
|
).map((call) => call[1])
|
||||||
const retryReadyNotification = notifications.find((notification) => notification.includes("[BACKGROUND TASK RETRY SESSION READY]"))
|
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`
|
const expectedRetryLink = `http://127.0.0.1:4096/${Buffer.from(tmpdir()).toString("base64url")}/session/ses_retry_created`
|
||||||
expect(retryReadyNotification).toBeDefined()
|
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 () => {
|
test("builds retry-ready links from the parent session directory when it differs from the manager directory", async () => {
|
||||||
//#given
|
//#given
|
||||||
const queuePendingNotification = mock(() => {})
|
const queuePendingParentWake = mock(() => {})
|
||||||
const managerDirectory = "/manager/dir"
|
const managerDirectory = "/manager/dir"
|
||||||
const parentDirectory = "/parent/dir"
|
const parentDirectory = "/parent/dir"
|
||||||
const client = {
|
const client = {
|
||||||
@@ -649,8 +665,14 @@ describe("BackgroundManager retry observability", () => {
|
|||||||
}
|
}
|
||||||
const manager = new BackgroundManager({ pluginContext: createPluginInput(client, managerDirectory) })
|
const manager = new BackgroundManager({ pluginContext: createPluginInput(client, managerDirectory) })
|
||||||
;(cast<{
|
;(cast<{
|
||||||
queuePendingNotification: (sessionId: string | undefined, notification: string) => void
|
queuePendingParentWake: (
|
||||||
}>(manager)).queuePendingNotification = queuePendingNotification
|
sessionId: string,
|
||||||
|
notification: string,
|
||||||
|
promptContext: Record<string, unknown>,
|
||||||
|
shouldReply: boolean,
|
||||||
|
delayMs?: number,
|
||||||
|
) => void
|
||||||
|
}>(manager)).queuePendingParentWake = queuePendingParentWake
|
||||||
const task = createMockTask({
|
const task = createMockTask({
|
||||||
id: "bg_retry_ready_parent_dir",
|
id: "bg_retry_ready_parent_dir",
|
||||||
parentSessionId: "parent-session",
|
parentSessionId: "parent-session",
|
||||||
@@ -699,9 +721,11 @@ describe("BackgroundManager retry observability", () => {
|
|||||||
}>(manager)).startTask({ task, input: taskInput, attemptID: "att_retry_ready_parent_dir" })
|
}>(manager)).startTask({ task, input: taskInput, attemptID: "att_retry_ready_parent_dir" })
|
||||||
|
|
||||||
//#then
|
//#then
|
||||||
const retryReadyNotification = cast<Array<[string | undefined, string]>>(queuePendingNotification.mock.calls)
|
const retryReadyNotification = cast<Array<[string, string, Record<string, unknown>, boolean, number | undefined]>>(
|
||||||
.map((call) => call[1])
|
queuePendingParentWake.mock.calls,
|
||||||
.find((notification) => notification.includes("[BACKGROUND TASK RETRY SESSION READY]"))
|
)
|
||||||
|
.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`
|
const expectedRetryLink = `http://127.0.0.1:4096/${Buffer.from(parentDirectory).toString("base64url")}/session/ses_retry_created_parent_dir`
|
||||||
expect(retryReadyNotification).toBeDefined()
|
expect(retryReadyNotification).toBeDefined()
|
||||||
expect(retryReadyNotification).toContain(expectedRetryLink)
|
expect(retryReadyNotification).toContain(expectedRetryLink)
|
||||||
@@ -1566,10 +1590,10 @@ describe("BackgroundManager.notifyParentSession - aborted parent", () => {
|
|||||||
await waitForCoalescedFlush()
|
await waitForCoalescedFlush()
|
||||||
|
|
||||||
//#then
|
//#then
|
||||||
const queuedNotifications = getPendingNotifications(manager).get("session-parent") ?? []
|
const pendingWake = getPendingParentWakes(manager).get("session-parent")
|
||||||
expect(queuedNotifications).toHaveLength(1)
|
expect(pendingWake?.notifications).toHaveLength(1)
|
||||||
expect(queuedNotifications[0]).toContain("<system-reminder>")
|
expect(pendingWake?.notifications[0]).toContain("<system-reminder>")
|
||||||
expect(queuedNotifications[0]).toContain("[ALL BACKGROUND TASKS COMPLETE]")
|
expect(pendingWake?.notifications[0]).toContain("[ALL BACKGROUND TASKS COMPLETE]")
|
||||||
|
|
||||||
manager.shutdown()
|
manager.shutdown()
|
||||||
})
|
})
|
||||||
@@ -1728,7 +1752,7 @@ describe("BackgroundManager.notifyParentSession - variant propagation", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
describe("BackgroundManager.injectPendingNotificationsIntoChatMessage", () => {
|
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
|
// given
|
||||||
const manager = createBackgroundManager()
|
const manager = createBackgroundManager()
|
||||||
manager.queuePendingNotification("session-parent", "<system-reminder>queued-one</system-reminder>")
|
manager.queuePendingNotification("session-parent", "<system-reminder>queued-one</system-reminder>")
|
||||||
@@ -1741,9 +1765,11 @@ describe("BackgroundManager.injectPendingNotificationsIntoChatMessage", () => {
|
|||||||
manager.injectPendingNotificationsIntoChatMessage(output, "session-parent")
|
manager.injectPendingNotificationsIntoChatMessage(output, "session-parent")
|
||||||
|
|
||||||
// then
|
// then
|
||||||
expect(output.parts[0].text).toContain("<system-reminder>queued-one</system-reminder>")
|
expect(output.parts).toEqual([{ type: "text", text: "User prompt" }])
|
||||||
expect(output.parts[0].text).toContain("<system-reminder>queued-two</system-reminder>")
|
expect(getPendingParentWakes(manager).get("session-parent")?.notifications).toEqual([
|
||||||
expect(output.parts[0].text).toContain("User prompt")
|
"<system-reminder>queued-one</system-reminder>\n\n<system-reminder>queued-two</system-reminder>",
|
||||||
|
])
|
||||||
|
expect(getPendingParentWakes(manager).get("session-parent")?.shouldReply).toBe(false)
|
||||||
expect(getPendingNotifications(manager).get("session-parent")).toBeUndefined()
|
expect(getPendingNotifications(manager).get("session-parent")).toBeUndefined()
|
||||||
|
|
||||||
manager.shutdown()
|
manager.shutdown()
|
||||||
|
|||||||
@@ -818,7 +818,7 @@ export class BackgroundManager {
|
|||||||
? `\n- Error: ${failedError}`
|
? `\n- Error: ${failedError}`
|
||||||
: ""
|
: ""
|
||||||
const retryModel = formatAttemptModelSummary(boundAttempt) ?? task.retryNotification.nextModel
|
const retryModel = formatAttemptModelSummary(boundAttempt) ?? task.retryNotification.nextModel
|
||||||
this.queuePendingNotification(
|
this.queuePendingParentWake(
|
||||||
task.parentSessionId,
|
task.parentSessionId,
|
||||||
`<system-reminder>
|
`<system-reminder>
|
||||||
[BACKGROUND TASK RETRY SESSION READY]
|
[BACKGROUND TASK RETRY SESSION READY]
|
||||||
@@ -829,7 +829,10 @@ export class BackgroundManager {
|
|||||||
**Retry link:** ${retrySessionUrl}${failedSessionLine}${failedModelLine}${failedErrorLine}${retryModel ? `\n- Model: \`${retryModel}\`` : ""}
|
**Retry link:** ${retrySessionUrl}${failedSessionLine}${failedModelLine}${failedErrorLine}${retryModel ? `\n- Model: \`${retryModel}\`` : ""}
|
||||||
|
|
||||||
The fallback retry session is now created and can be inspected directly.
|
The fallback retry session is now created and can be inspected directly.
|
||||||
</system-reminder>`
|
</system-reminder>`,
|
||||||
|
{},
|
||||||
|
false,
|
||||||
|
PENDING_PARENT_WAKE_DEBOUNCE_MS,
|
||||||
)
|
)
|
||||||
task.retryNotification = undefined
|
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 failedModelLine = failedModel ? `\n- Failed model: \`${failedModel}\`` : ""
|
||||||
const failedErrorLine = previousAttempt?.error ? `\n- Error: ${previousAttempt.error}` : ""
|
const failedErrorLine = previousAttempt?.error ? `\n- Error: ${previousAttempt.error}` : ""
|
||||||
const nextModel = formatAttemptModelSummary(currentAttempt)
|
const nextModel = formatAttemptModelSummary(currentAttempt)
|
||||||
this.queuePendingNotification(
|
this.queuePendingParentWake(
|
||||||
task.parentSessionId,
|
task.parentSessionId,
|
||||||
`<system-reminder>
|
`<system-reminder>
|
||||||
[BACKGROUND TASK RETRYING]
|
[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}\`` : ""}
|
**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.
|
The task was re-queued on a fallback model after a retryable failure.
|
||||||
</system-reminder>`
|
</system-reminder>`,
|
||||||
|
{},
|
||||||
|
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)
|
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)
|
const pendingNotifications = this.pendingNotifications.get(sessionID)
|
||||||
if (!pendingNotifications || pendingNotifications.length === 0) {
|
if (!pendingNotifications || pendingNotifications.length === 0) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
this.pendingNotifications.delete(sessionID)
|
|
||||||
const notificationContent = pendingNotifications.join("\n\n")
|
const notificationContent = pendingNotifications.join("\n\n")
|
||||||
const firstTextPartIndex = output.parts.findIndex((part) => part.type === "text")
|
this.pendingNotifications.delete(sessionID)
|
||||||
|
this.queuePendingParentWake(sessionID, notificationContent, {}, false, PENDING_PARENT_WAKE_DEBOUNCE_MS)
|
||||||
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}`
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -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 })
|
log("[background-agent] Sent deferred parent wake:", { sessionID })
|
||||||
this.trackDispatchedParentWake(sessionID, latestWake)
|
this.trackDispatchedParentWake(sessionID, latestWake)
|
||||||
} catch (error) {
|
} 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 })
|
log("[background-agent] Failed to send deferred parent wake:", { sessionID, error })
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -28,12 +28,6 @@ const FORWARDED_EVENT_TYPES = new Set([
|
|||||||
"session.status",
|
"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) {
|
export function createBackgroundNotificationHook(manager: BackgroundManager) {
|
||||||
const eventHandler = async ({ event }: EventInput) => {
|
const eventHandler = async ({ event }: EventInput) => {
|
||||||
if (!FORWARDED_EVENT_TYPES.has(event.type)) return
|
if (!FORWARDED_EVENT_TYPES.has(event.type)) return
|
||||||
|
|||||||
Reference in New Issue
Block a user