From 023993249f352dd4875df584479012cbac7f80c2 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 19 Jun 2026 22:03:48 +0100 Subject: [PATCH] fix(queue): restart dormant followup drains (#95039) Merged via squash. Prepared head SHA: b6a81f07f16da50a513285a8572f53794c329b65 Co-authored-by: steipete <58493+steipete@users.noreply.github.com> Reviewed-by: @steipete --- .../agent-runner.runreplyagent.e2e.test.ts | 33 ++++++++++- src/auto-reply/reply/agent-runner.ts | 59 ++++++++++++------- 2 files changed, 70 insertions(+), 22 deletions(-) diff --git a/src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts b/src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts index b46e726e1ffa..0c1e69623c35 100644 --- a/src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts +++ b/src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts @@ -424,6 +424,11 @@ describe("runReplyAgent heartbeat followup guard", () => { }); it("keeps typing alive when a followup is queued behind a live active run", async () => { + const active = createReplyOperation({ + sessionKey: "main", + sessionId: "session", + resetTriggered: false, + }); const { run, typing } = createMinimalRun({ opts: { isHeartbeat: false }, isActive: true, @@ -441,9 +446,10 @@ describe("runReplyAgent heartbeat followup guard", () => { expect(typing.startTypingLoop).toHaveBeenCalledTimes(1); expect(typing.refreshTypingTtl).toHaveBeenCalledTimes(1); expect(typing.cleanup).not.toHaveBeenCalled(); + active.complete(); }); - it("starts draining immediately when the active snapshot is already stale", async () => { + it("starts draining after enqueue when the reply lane owner is already gone", async () => { const { run, typing } = createMinimalRun({ opts: { isHeartbeat: false }, isActive: true, @@ -456,11 +462,36 @@ describe("runReplyAgent heartbeat followup guard", () => { expect(result).toBeUndefined(); expect(vi.mocked(enqueueFollowupRun)).toHaveBeenCalledTimes(1); + expect(vi.mocked(enqueueFollowupRun).mock.calls[0]?.[5]).toBe(false); expect(vi.mocked(scheduleFollowupDrain)).toHaveBeenCalledTimes(1); expect(state.runEmbeddedAgentMock).not.toHaveBeenCalled(); expect(typing.cleanup).toHaveBeenCalledTimes(1); }); + it("keeps the drain dormant until the reply lane owner clears", async () => { + const active = createReplyOperation({ + sessionKey: "main", + sessionId: "session", + resetTriggered: false, + }); + const { run } = createMinimalRun({ + opts: { isHeartbeat: false }, + isActive: true, + isRunActive: () => true, + shouldFollowup: true, + resolvedQueueMode: "collect", + }); + + await run(); + + expect(vi.mocked(enqueueFollowupRun).mock.calls[0]?.[5]).toBe(false); + expect(vi.mocked(scheduleFollowupDrain)).not.toHaveBeenCalled(); + + active.complete(); + + expect(vi.mocked(scheduleFollowupDrain)).toHaveBeenCalledTimes(1); + }); + it("drains followup queue when an unexpected exception escapes the run path", async () => { const accounting = await import("./session-run-accounting.js"); const persistSpy = vi diff --git a/src/auto-reply/reply/agent-runner.ts b/src/auto-reply/reply/agent-runner.ts index 639f7d9c44c0..74e33f279f78 100644 --- a/src/auto-reply/reply/agent-runner.ts +++ b/src/auto-reply/reply/agent-runner.ts @@ -135,6 +135,26 @@ import type { TypingController } from "./typing.js"; const BLOCK_REPLY_SEND_TIMEOUT_MS = 15_000; +function scheduleFollowupDrainAfterReplyOperationClear(params: { + operation: ReplyOperation; + queueKey: string; + runFollowup: (run: FollowupRun) => Promise; +}): void { + runAfterReplyOperationClear(params.operation, (admissionSessionId) => { + const completedSessionId = params.operation.sessionId; + const runFollowupAfterClear = + admissionSessionId === completedSessionId + ? params.runFollowup + : (queued: FollowupRun) => + params.runFollowup( + queued.run.sessionId === completedSessionId + ? { ...queued, admissionSessionId } + : queued, + ); + scheduleFollowupDrain(params.queueKey, runFollowupAfterClear); + }); +} + function markBeforeAgentRunBlockedPayloads(payloads: ReplyPayload[]): ReplyPayload[] { return payloads.map((payload) => setReplyPayloadMetadata(payload, { beforeAgentRunBlocked: true }), @@ -1317,12 +1337,19 @@ export async function runReplyAgent(params: { typing.cleanup(); return undefined; } - // Re-check liveness after enqueue so a stale active snapshot cannot leave - // the followup queue idle if the original run already finished. - const queuedBehindActiveRun = isRunActive?.() === true; - if (!queuedBehindActiveRun) { + // The queue must stay dormant while the active owner can still collect + // messages. Registering after enqueue closes the owner-clear race. + const activeReplyOperation = replyRunRegistry.get(queueKey); + if (activeReplyOperation) { + scheduleFollowupDrainAfterReplyOperationClear({ + operation: activeReplyOperation, + queueKey, + runFollowup: queuedRunFollowupTurn, + }); + } else { scheduleFollowupDrain(queueKey, queuedRunFollowupTurn); } + const queuedBehindActiveRun = isRunActive?.() === true; await touchActiveSessionEntry(); if (queuedBehindActiveRun) { await typingSignals.signalToolStart(); @@ -1464,19 +1491,6 @@ export async function runReplyAgent(params: { shouldDrainQueuedFollowupsAfterClear = true; return value; }; - const drainQueuedFollowupsAfterClear = (admissionSessionId: string) => { - const completedSessionId = replyOperation.sessionId; - const runFollowupAfterClear = - admissionSessionId === completedSessionId - ? runFollowupTurn - : (queued: FollowupRun) => - runFollowupTurn( - queued.run.sessionId === completedSessionId - ? { ...queued, admissionSessionId } - : queued, - ); - scheduleFollowupDrain(queueKey, runFollowupAfterClear); - }; const restartRecoveryDeliveryRunId = crypto.randomUUID(); let trackedRestartRecoveryDeliveryContext = false; const persistRestartRecoveryDeliveryContext = async (): Promise => { @@ -2625,10 +2639,13 @@ export async function runReplyAgent(params: { ); } if (shouldDrainQueuedFollowupsAfterClear) { - if (providedReplyOperation) { - runAfterReplyOperationClear(replyOperation, drainQueuedFollowupsAfterClear); - } else { - replyOperation.completeThen(() => drainQueuedFollowupsAfterClear(replyOperation.sessionId)); + scheduleFollowupDrainAfterReplyOperationClear({ + operation: replyOperation, + queueKey, + runFollowup: runFollowupTurn, + }); + if (!providedReplyOperation) { + replyOperation.complete(); } } else if (!providedReplyOperation) { replyOperation.complete();