fix(background-agent): defer busy parent notifications
This commit is contained in:
@@ -107,6 +107,7 @@ type ParentWakePromptContext = {
|
|||||||
type PendingParentWake = {
|
type PendingParentWake = {
|
||||||
promptContext: ParentWakePromptContext
|
promptContext: ParentWakePromptContext
|
||||||
notifications: string[]
|
notifications: string[]
|
||||||
|
shouldReply: boolean
|
||||||
}
|
}
|
||||||
|
|
||||||
const PENDING_PARENT_WAKE_RETRY_MS = 1_000
|
const PENDING_PARENT_WAKE_RETRY_MS = 1_000
|
||||||
@@ -2235,14 +2236,15 @@ The task was re-queued on a fallback model after a retryable failure.
|
|||||||
...(variant !== undefined ? { variant } : {}),
|
...(variant !== undefined ? { variant } : {}),
|
||||||
...(resolvedTools ? { tools: resolvedTools } : {}),
|
...(resolvedTools ? { tools: resolvedTools } : {}),
|
||||||
}
|
}
|
||||||
const shouldDeferReply = shouldReply && await this.isSessionActive(task.parentSessionId)
|
const shouldDeferNotification = await this.isSessionActive(task.parentSessionId)
|
||||||
|
|
||||||
if (shouldDeferReply) {
|
if (shouldDeferNotification) {
|
||||||
this.queuePendingParentWake(task.parentSessionId, notification, parentPromptContext)
|
this.queuePendingParentWake(task.parentSessionId, notification, parentPromptContext, shouldReply)
|
||||||
log("[background-agent] Deferred notification until parent session is idle:", {
|
log("[background-agent] Deferred notification until parent session is idle:", {
|
||||||
taskId: task.id,
|
taskId: task.id,
|
||||||
allComplete,
|
allComplete,
|
||||||
isTaskFailure,
|
isTaskFailure,
|
||||||
|
shouldReply,
|
||||||
})
|
})
|
||||||
} else {
|
} else {
|
||||||
try {
|
try {
|
||||||
@@ -2292,15 +2294,18 @@ The task was re-queued on a fallback model after a retryable failure.
|
|||||||
sessionID: string,
|
sessionID: string,
|
||||||
notification: string,
|
notification: string,
|
||||||
promptContext: ParentWakePromptContext,
|
promptContext: ParentWakePromptContext,
|
||||||
|
shouldReply: boolean,
|
||||||
): void {
|
): void {
|
||||||
const pendingWake = this.pendingParentWakes.get(sessionID)
|
const pendingWake = this.pendingParentWakes.get(sessionID)
|
||||||
if (pendingWake) {
|
if (pendingWake) {
|
||||||
pendingWake.notifications.push(notification)
|
pendingWake.notifications.push(notification)
|
||||||
pendingWake.promptContext = promptContext
|
pendingWake.promptContext = promptContext
|
||||||
|
pendingWake.shouldReply = pendingWake.shouldReply || shouldReply
|
||||||
} else {
|
} else {
|
||||||
this.pendingParentWakes.set(sessionID, {
|
this.pendingParentWakes.set(sessionID, {
|
||||||
promptContext,
|
promptContext,
|
||||||
notifications: [notification],
|
notifications: [notification],
|
||||||
|
shouldReply,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
this.schedulePendingParentWakeFlush(sessionID)
|
this.schedulePendingParentWakeFlush(sessionID)
|
||||||
@@ -2334,7 +2339,7 @@ The task was re-queued on a fallback model after a retryable failure.
|
|||||||
await promptAsyncInDirectory(this.client, {
|
await promptAsyncInDirectory(this.client, {
|
||||||
path: { id: sessionID },
|
path: { id: sessionID },
|
||||||
body: {
|
body: {
|
||||||
noReply: false,
|
noReply: !pendingWake.shouldReply,
|
||||||
...pendingWake.promptContext,
|
...pendingWake.promptContext,
|
||||||
parts: [createInternalAgentTextPart(notificationContent)],
|
parts: [createInternalAgentTextPart(notificationContent)],
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -281,6 +281,76 @@ describe("BackgroundManager.notifyParentSession cleanup scheduling", () => {
|
|||||||
expect(promptAsyncCalls).toHaveLength(0)
|
expect(promptAsyncCalls).toHaveLength(0)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("#when partial completion arrives while parent session is busy #then notification waits until idle without waking a reply", async () => {
|
||||||
|
// given
|
||||||
|
const sessionStatuses: Record<string, { type: string }> = {
|
||||||
|
"parent-1": { type: "busy" },
|
||||||
|
}
|
||||||
|
const { manager, promptAsyncCalls } = createManager(true, sessionStatuses)
|
||||||
|
managerUnderTest = manager
|
||||||
|
const taskA = createTask({ id: "task-a", parentSessionId: "parent-1", description: "task A", status: "completed", completedAt: new Date("2026-03-11T00:01:00.000Z") })
|
||||||
|
const taskB = createTask({ id: "task-b", parentSessionId: "parent-1", description: "task B", status: "running" })
|
||||||
|
getTasks(manager).set(taskA.id, taskA)
|
||||||
|
getTasks(manager).set(taskB.id, taskB)
|
||||||
|
getPendingByParent(manager).set(taskA.parentSessionId, new Set([taskA.id, taskB.id]))
|
||||||
|
|
||||||
|
// when
|
||||||
|
await notifyParentSessionForTest(manager, taskA)
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(promptAsyncCalls).toHaveLength(0)
|
||||||
|
|
||||||
|
// when
|
||||||
|
sessionStatuses["parent-1"] = { type: "idle" }
|
||||||
|
manager.handleEvent({ type: "session.idle", properties: { sessionID: "parent-1" } })
|
||||||
|
await waitForDeferredWake()
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(promptAsyncCalls).toHaveLength(1)
|
||||||
|
expect(promptAsyncCalls[0]?.body.noReply).toBe(true)
|
||||||
|
const notificationPayload = JSON.stringify(promptAsyncCalls[0]?.body.parts)
|
||||||
|
expect(notificationPayload).toContain("BACKGROUND TASK COMPLETED")
|
||||||
|
expect(notificationPayload).not.toContain("ALL BACKGROUND TASKS COMPLETE")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("#when partial and all-complete notifications queue while parent session is busy #then idle flushes one reply wake", async () => {
|
||||||
|
// given
|
||||||
|
const sessionStatuses: Record<string, { type: string }> = {
|
||||||
|
"parent-1": { type: "busy" },
|
||||||
|
}
|
||||||
|
const { manager, promptAsyncCalls } = createManager(true, sessionStatuses)
|
||||||
|
managerUnderTest = manager
|
||||||
|
const taskA = createTask({ id: "task-a", parentSessionId: "parent-1", description: "task A", status: "completed", completedAt: new Date("2026-03-11T00:01:00.000Z") })
|
||||||
|
const taskB = createTask({ id: "task-b", parentSessionId: "parent-1", description: "task B", status: "running" })
|
||||||
|
getTasks(manager).set(taskA.id, taskA)
|
||||||
|
getTasks(manager).set(taskB.id, taskB)
|
||||||
|
getPendingByParent(manager).set(taskA.parentSessionId, new Set([taskA.id, taskB.id]))
|
||||||
|
|
||||||
|
await notifyParentSessionForTest(manager, taskA)
|
||||||
|
taskB.status = "completed"
|
||||||
|
taskB.completedAt = new Date("2026-03-11T00:02:00.000Z")
|
||||||
|
|
||||||
|
// when
|
||||||
|
await notifyParentSessionForTest(manager, taskB)
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(promptAsyncCalls).toHaveLength(0)
|
||||||
|
|
||||||
|
// when
|
||||||
|
sessionStatuses["parent-1"] = { type: "idle" }
|
||||||
|
manager.handleEvent({ type: "session.idle", properties: { sessionID: "parent-1" } })
|
||||||
|
await waitForDeferredWake()
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(promptAsyncCalls).toHaveLength(1)
|
||||||
|
expect(promptAsyncCalls[0]?.body.noReply).toBe(false)
|
||||||
|
const notificationPayload = JSON.stringify(promptAsyncCalls[0]?.body.parts)
|
||||||
|
expect(notificationPayload).toContain("BACKGROUND TASK COMPLETED")
|
||||||
|
expect(notificationPayload).toContain("ALL BACKGROUND TASKS COMPLETE")
|
||||||
|
expect(notificationPayload).toContain(taskA.id)
|
||||||
|
expect(notificationPayload).toContain(taskB.id)
|
||||||
|
})
|
||||||
|
|
||||||
test("#when all-complete notification wakes parent #then prompt stays in the same OpenCode directory instance", async () => {
|
test("#when all-complete notification wakes parent #then prompt stays in the same OpenCode directory instance", async () => {
|
||||||
// given
|
// given
|
||||||
const { manager, promptAsyncCalls } = createManager(true)
|
const { manager, promptAsyncCalls } = createManager(true)
|
||||||
|
|||||||
Reference in New Issue
Block a user