Merge pull request #4478 from code-yeongyu/fix-4164-bg-reminder-timing
fix(parent-wake-notifier): deliver background completion reminders during active parent turn (#4164)
This commit is contained in:
@@ -2449,8 +2449,14 @@ The task was re-queued on a fallback model after a retryable failure.
|
|||||||
const shouldDeferNotification = await this.isSessionActive(task.parentSessionId)
|
const shouldDeferNotification = await this.isSessionActive(task.parentSessionId)
|
||||||
|
|
||||||
if (shouldDeferNotification) {
|
if (shouldDeferNotification) {
|
||||||
this.queuePendingParentWake(task.parentSessionId, notification, parentPromptContext, shouldReply)
|
this.queuePendingParentWake(
|
||||||
log("[background-agent] Deferred notification until parent session is idle:", {
|
task.parentSessionId,
|
||||||
|
notification,
|
||||||
|
parentPromptContext,
|
||||||
|
shouldReply,
|
||||||
|
PENDING_PARENT_WAKE_DEBOUNCE_MS,
|
||||||
|
)
|
||||||
|
log("[background-agent] Queued notification while parent session is active:", {
|
||||||
taskId: task.id,
|
taskId: task.id,
|
||||||
allComplete,
|
allComplete,
|
||||||
isTaskFailure,
|
isTaskFailure,
|
||||||
|
|||||||
@@ -108,6 +108,61 @@ async function flushPendingParentWakeForTest(manager: BackgroundManager, session
|
|||||||
}
|
}
|
||||||
|
|
||||||
describe("BackgroundManager parent wake active turn events", () => {
|
describe("BackgroundManager parent wake active turn events", () => {
|
||||||
|
test("#when background task completes during active parent turn #then parent gets same-turn no-reply reminder", async () => {
|
||||||
|
// given
|
||||||
|
const sessionStatuses: Record<string, { type: string }> = {
|
||||||
|
"parent-1": { type: "busy" },
|
||||||
|
}
|
||||||
|
const { manager, promptAsyncCalls } = createManager(sessionStatuses)
|
||||||
|
managerUnderTest = manager
|
||||||
|
const task = createTask({
|
||||||
|
id: "task-a",
|
||||||
|
parentSessionId: "parent-1",
|
||||||
|
description: "task A",
|
||||||
|
status: "completed",
|
||||||
|
completedAt: new Date("2026-05-20T14:19:14.625Z"),
|
||||||
|
})
|
||||||
|
getTasks(manager).set(task.id, task)
|
||||||
|
getPendingByParent(manager).set(task.parentSessionId, new Set([task.id]))
|
||||||
|
|
||||||
|
// when
|
||||||
|
await notifyParentSessionForTest(manager, task)
|
||||||
|
await flushPendingParentWakeForTest(manager, "parent-1")
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(promptAsyncCalls).toHaveLength(1)
|
||||||
|
expect(promptAsyncCalls[0]?.body.noReply).toBe(true)
|
||||||
|
expect(JSON.stringify(promptAsyncCalls[0]?.body.parts)).toContain("ALL BACKGROUND TASKS COMPLETE")
|
||||||
|
expect(getPendingParentWakes(manager).has("parent-1")).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
test("#when background task fails during active parent turn #then parent wake stays deferred", async () => {
|
||||||
|
// given
|
||||||
|
const sessionStatuses: Record<string, { type: string }> = {
|
||||||
|
"parent-1": { type: "busy" },
|
||||||
|
}
|
||||||
|
const { manager, promptAsyncCalls } = createManager(sessionStatuses)
|
||||||
|
managerUnderTest = manager
|
||||||
|
const task = createTask({
|
||||||
|
id: "task-a",
|
||||||
|
parentSessionId: "parent-1",
|
||||||
|
description: "task A",
|
||||||
|
status: "error",
|
||||||
|
error: "UnknownError: UnknownError",
|
||||||
|
completedAt: new Date("2026-05-20T14:19:14.625Z"),
|
||||||
|
})
|
||||||
|
getTasks(manager).set(task.id, task)
|
||||||
|
getPendingByParent(manager).set(task.parentSessionId, new Set([task.id]))
|
||||||
|
|
||||||
|
// when
|
||||||
|
await notifyParentSessionForTest(manager, task)
|
||||||
|
await flushPendingParentWakeForTest(manager, "parent-1")
|
||||||
|
|
||||||
|
// then
|
||||||
|
expect(promptAsyncCalls).toHaveLength(0)
|
||||||
|
expect(getPendingParentWakes(manager).has("parent-1")).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
test("#when parent reasoning delta is newer than stale idle state #then background completion does not fork a reply", async () => {
|
test("#when parent reasoning delta is newer than stale idle state #then background completion does not fork a reply", async () => {
|
||||||
// given
|
// given
|
||||||
const sessionStatuses: Record<string, { type: string }> = {
|
const sessionStatuses: Record<string, { type: string }> = {
|
||||||
|
|||||||
@@ -77,6 +77,19 @@ type ToolWaitDeferralDecision = {
|
|||||||
|
|
||||||
type Unrefable = ReturnType<typeof setTimeout> & { unref?: () => unknown }
|
type Unrefable = ReturnType<typeof setTimeout> & { unref?: () => unknown }
|
||||||
|
|
||||||
|
const ACTIVE_TURN_COMPLETION_NOTIFICATION_MARKERS = [
|
||||||
|
"[BACKGROUND TASK COMPLETED]",
|
||||||
|
"[ALL BACKGROUND TASKS COMPLETE]",
|
||||||
|
] as const
|
||||||
|
|
||||||
|
function notificationAllowsActiveTurnDelivery(notification: string): boolean {
|
||||||
|
return ACTIVE_TURN_COMPLETION_NOTIFICATION_MARKERS.some((marker) => notification.includes(marker))
|
||||||
|
}
|
||||||
|
|
||||||
|
function pendingWakeAllowsActiveTurnDelivery(wake: PendingParentWake): boolean {
|
||||||
|
return wake.notifications.length > 0 && wake.notifications.every(notificationAllowsActiveTurnDelivery)
|
||||||
|
}
|
||||||
|
|
||||||
function unrefTimerHandle(handle: ReturnType<typeof setTimeout>): void {
|
function unrefTimerHandle(handle: ReturnType<typeof setTimeout>): void {
|
||||||
const maybeUnref = (handle as Unrefable).unref
|
const maybeUnref = (handle as Unrefable).unref
|
||||||
if (typeof maybeUnref === "function") {
|
if (typeof maybeUnref === "function") {
|
||||||
@@ -151,23 +164,26 @@ export class ParentWakeNotifier {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
if (await this.isSessionActive(sessionID)) {
|
const sessionActive = await this.isSessionActive(sessionID)
|
||||||
this.schedulePendingParentWakeFlush(sessionID)
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
this.clearPendingParentWakeTimer(sessionID)
|
this.clearPendingParentWakeTimer(sessionID)
|
||||||
await settleAfterSessionIdle()
|
if (!sessionActive) {
|
||||||
|
await settleAfterSessionIdle()
|
||||||
|
|
||||||
if (await this.isSessionActive(sessionID)) {
|
if (await this.isSessionActive(sessionID)) {
|
||||||
this.schedulePendingParentWakeFlush(sessionID)
|
this.schedulePendingParentWakeFlush(sessionID)
|
||||||
return
|
return
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const latestWake = this.pendingParentWakes.get(sessionID)
|
const latestWake = this.pendingParentWakes.get(sessionID)
|
||||||
if (!latestWake) {
|
if (!latestWake) {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
const canDeliverDuringActiveTurn = sessionActive && pendingWakeAllowsActiveTurnDelivery(latestWake)
|
||||||
|
if (sessionActive && !canDeliverDuringActiveTurn) {
|
||||||
|
this.schedulePendingParentWakeFlush(sessionID)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
if (this.hasRecentParentSessionActivity(sessionID)) {
|
if (this.hasRecentParentSessionActivity(sessionID)) {
|
||||||
this.schedulePendingParentWakeFlush(sessionID)
|
this.schedulePendingParentWakeFlush(sessionID)
|
||||||
@@ -218,11 +234,12 @@ export class ParentWakeNotifier {
|
|||||||
source: "background-agent-parent-wake",
|
source: "background-agent-parent-wake",
|
||||||
settleMs: 0,
|
settleMs: 0,
|
||||||
queueBehavior: "defer",
|
queueBehavior: "defer",
|
||||||
|
checkStatus: !canDeliverDuringActiveTurn,
|
||||||
checkToolState: !toolWaitDecision.skipPromptGateToolStateCheck,
|
checkToolState: !toolWaitDecision.skipPromptGateToolStateCheck,
|
||||||
input: {
|
input: {
|
||||||
path: { id: sessionID },
|
path: { id: sessionID },
|
||||||
body: {
|
body: {
|
||||||
noReply: !latestWake.shouldReply,
|
noReply: canDeliverDuringActiveTurn ? true : !latestWake.shouldReply,
|
||||||
...latestWake.promptContext,
|
...latestWake.promptContext,
|
||||||
parts: [createInternalAgentTextPart(notificationContent)],
|
parts: [createInternalAgentTextPart(notificationContent)],
|
||||||
},
|
},
|
||||||
|
|||||||
Reference in New Issue
Block a user