From 1b8d8376744598281b3877eb1359e7d8dafa623d Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 4 Jul 2026 21:15:28 -0400 Subject: [PATCH] fix(tui): preserve queued prompt lifecycle (#100123) Allow busy TUI sessions to forward prompts into the configured queue while keeping queued-turn admission, cancellation, restart, and transcript ownership consistent across the TUI, Gateway, and followup queue. Co-authored-by: Sebastien Tardif --- .../OpenClawProtocol/GatewayModels.swift | 6 +- docs/concepts/queue.md | 22 + docs/docs_map.md | 1 + packages/gateway-protocol/src/index.test.ts | 1 + .../gateway-protocol/src/schema/logs-chat.ts | 1 + scripts/protocol-gen-swift.ts | 4 +- src/auto-reply/get-reply-options.types.ts | 9 +- .../reply/dispatch-from-config.test.ts | 42 ++ src/auto-reply/reply/dispatch-from-config.ts | 12 +- src/auto-reply/reply/followup-runner.test.ts | 29 + src/auto-reply/reply/followup-runner.ts | 11 +- src/auto-reply/reply/get-reply-run.ts | 9 +- src/auto-reply/reply/queue.collect.test.ts | 683 +++++++++++++++++- .../reply/queue.drain-restart.test.ts | 46 +- src/auto-reply/reply/queue/drain.ts | 636 ++++++++++++---- src/auto-reply/reply/queue/enqueue.ts | 41 +- src/auto-reply/reply/queue/state.test.ts | 27 +- src/auto-reply/reply/queue/state.ts | 58 +- src/auto-reply/reply/queue/types.ts | 19 +- src/gateway/chat-abort.ts | 4 + src/gateway/chat-queued-turns.test.ts | 222 ++++++ src/gateway/chat-queued-turns.ts | 214 ++++++ src/gateway/local-request-context.ts | 1 + src/gateway/server-close.test.ts | 35 + src/gateway/server-close.ts | 41 +- src/gateway/server-maintenance.test.ts | 46 ++ src/gateway/server-maintenance.ts | 11 +- src/gateway/server-methods/agent.test.ts | 1 + .../chat.abort-authorization.test.ts | 236 +++++- .../server-methods/chat.abort.test-helpers.ts | 11 +- .../chat.directive-tags.test.ts | 2 + src/gateway/server-methods/chat.ts | 258 ++++++- src/gateway/server-methods/shared-types.ts | 2 + src/gateway/server-request-context.test.ts | 1 + src/gateway/server-request-context.ts | 2 + src/gateway/server-runtime-state.ts | 3 + src/gateway/server-startup-early.ts | 2 + .../server-startup-post-attach.test.ts | 1 + .../server.chat.gateway-server-chat-b.test.ts | 218 ++++++ src/gateway/server.impl.ts | 4 + src/gateway/test-helpers.maintenance-state.ts | 1 + src/sessions/user-turn-transcript.ts | 17 +- src/tui/embedded-backend.test.ts | 62 +- src/tui/embedded-backend.ts | 38 +- src/tui/gateway-chat.test.ts | 46 ++ src/tui/gateway-chat.ts | 38 +- src/tui/tui-backend.ts | 7 +- src/tui/tui-command-handlers.test.ts | 165 ++++- src/tui/tui-command-handlers.ts | 29 +- src/tui/tui-pty-local.e2e.test.ts | 305 +++++++- src/tui/tui-session-actions.test.ts | 113 ++- src/tui/tui-session-actions.ts | 44 +- src/tui/tui-types.ts | 2 +- src/tui/tui.test.ts | 31 +- src/tui/tui.ts | 8 +- test/helpers/openclaw-test-instance.ts | 23 +- 56 files changed, 3522 insertions(+), 379 deletions(-) create mode 100644 src/gateway/chat-queued-turns.test.ts create mode 100644 src/gateway/chat-queued-turns.ts diff --git a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift index 7e468ee01bb..b7832625b18 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawProtocol/GatewayModels.swift @@ -7778,21 +7778,25 @@ public struct ChatAbortParams: Codable, Sendable { public let sessionkey: String public let agentid: String? public let runid: String? + public let preservesideruns: Bool? public init( sessionkey: String, agentid: String? = nil, - runid: String?) + runid: String?, + preservesideruns: Bool? = nil) { self.sessionkey = sessionkey self.agentid = agentid self.runid = runid + self.preservesideruns = preservesideruns } private enum CodingKeys: String, CodingKey { case sessionkey = "sessionKey" case agentid = "agentId" case runid = "runId" + case preservesideruns = "preserveSideRuns" } } diff --git a/docs/concepts/queue.md b/docs/concepts/queue.md index 93fe0c24f1a..8f5180254cb 100644 --- a/docs/concepts/queue.md +++ b/docs/concepts/queue.md @@ -118,6 +118,28 @@ keys. - Applies to auto-reply agent runs across all inbound channels that use the gateway reply pipeline (WhatsApp web, Telegram, Slack, Discord, Signal, iMessage, webchat, etc.). - Default lane (`main`) is process-wide for inbound + main heartbeats; set `agents.defaults.maxConcurrent` to allow multiple sessions in parallel. - Additional lanes may exist (e.g. `cron`, `cron-nested`, `nested`, `subagent`) so background jobs can run in parallel without blocking inbound replies. Isolated cron agent turns hold a `cron` slot while their inner agent execution uses `cron-nested`; both use `cron.maxConcurrentRuns`. Shared non-cron `nested` flows keep their own lane behavior. These detached runs are tracked as [background tasks](/automation/tasks). + +## Queued-turn cancellation + +When Gateway admits a prompt into the followup/collect queue (for example a TUI +or webchat `chat.send` while another turn is active), it keeps a **Gateway-owned +cancel identity** for that client `runId` until the queued content runs or is +dropped. The identity follows content folded into an overflow summary. + +- `chat.abort` with a specific `runId` cancels that turn while it is still queued, + if the requester is authorized (same ownership rules as active runs). +- `chat.abort` for a session without `runId` cancels **authorized queued turns + first**, then aborts authorized active runs. That order prevents queue drain + from promoting work into a half-stopped session. +- Clearing the entire session queue without per-requester checks is not the stop + path for multi-owner sessions. +- Queued waits are not projected as active agent runs for `sessions.list` and do + not own active-run timeout semantics; only the active phase does. + +Clients (including the TUI) forward mid-run prompts and let Gateway apply the +queue mode. Esc/`/stop` uses a session-scoped abort so lost local handles cannot +leave a still-queued prompt running. + - Per-session lanes guarantee that only one agent run touches a given session at a time. - No external dependencies or background worker threads; pure TypeScript + promises. diff --git a/docs/docs_map.md b/docs/docs_map.md index 54fe2d69204..fc4924e783a 100644 --- a/docs/docs_map.md +++ b/docs/docs_map.md @@ -2757,6 +2757,7 @@ Do not edit it by hand; run `pnpm docs:map:gen`. - H2: Precedence - H2: Per-session overrides - H2: Scope and guarantees + - H2: Queued-turn cancellation - H2: Troubleshooting - H2: Related diff --git a/packages/gateway-protocol/src/index.test.ts b/packages/gateway-protocol/src/index.test.ts index a404053c73c..b892f171f92 100644 --- a/packages/gateway-protocol/src/index.test.ts +++ b/packages/gateway-protocol/src/index.test.ts @@ -118,6 +118,7 @@ describe("lazy protocol validators", () => { sessionKey: "global", agentId: "work", runId: "run-global-work", + preserveSideRuns: true, }), ).toBe(true); expect( diff --git a/packages/gateway-protocol/src/schema/logs-chat.ts b/packages/gateway-protocol/src/schema/logs-chat.ts index bc49dacc299..c2ff41ba2a9 100644 --- a/packages/gateway-protocol/src/schema/logs-chat.ts +++ b/packages/gateway-protocol/src/schema/logs-chat.ts @@ -107,6 +107,7 @@ export const ChatAbortParamsSchema = Type.Object( sessionKey: NonEmptyString, agentId: Type.Optional(NonEmptyString), runId: Type.Optional(NonEmptyString), + preserveSideRuns: Type.Optional(Type.Boolean()), }, { additionalProperties: false }, ); diff --git a/scripts/protocol-gen-swift.ts b/scripts/protocol-gen-swift.ts index dd1a3ff9339..7da59627fcc 100644 --- a/scripts/protocol-gen-swift.ts +++ b/scripts/protocol-gen-swift.ts @@ -64,7 +64,7 @@ const DEFAULTED_OPTIONAL_INIT_PARAM_ENTRIES: readonly [string, readonly string[] ["SessionsUsageParams", ["agentId", "agentScope"]], ["ChatHistoryParams", ["agentId", "offset"]], ["ChatSendParams", ["agentId"]], - ["ChatAbortParams", ["agentId"]], + ["ChatAbortParams", ["agentId", "preserveSideRuns"]], ["ChatInjectParams", ["agentId"]], ["ChatDeltaEvent", ["agentId"]], ["ChatFinalEvent", ["agentId"]], @@ -73,7 +73,7 @@ const DEFAULTED_OPTIONAL_INIT_PARAM_ENTRIES: readonly [string, readonly string[] ["ArtifactsListParams", ["agentId"]], ["ArtifactsGetParams", ["agentId"]], ["ArtifactsDownloadParams", ["agentId"]], - ["ChatAbortParams", ["agentId"]], + ["ChatAbortParams", ["agentId", "preserveSideRuns"]], ["ChatAbortedEvent", ["agentId"]], ["ChatDeltaEvent", ["agentId"]], ["ChatErrorEvent", ["agentId"]], diff --git a/src/auto-reply/get-reply-options.types.ts b/src/auto-reply/get-reply-options.types.ts index 1b99cec9b6a..ed858a17952 100644 --- a/src/auto-reply/get-reply-options.types.ts +++ b/src/auto-reply/get-reply-options.types.ts @@ -43,7 +43,14 @@ export type QueuedReplyDeliveryCorrelation = { /** Lifecycle hooks for queued follow-up replies. */ export type QueuedReplyLifecycle = { - onEnqueued?: () => void; + /** Stable cancellation owner used to keep collect-mode batches authorization-safe. */ + ownerKey?: string; + /** Return false when the external owner rejects this queue identity. */ + onEnqueued?: () => boolean | void; + /** Retires this source's cancellation ownership while retaining its live identity. */ + onCancellationRetired?: () => void; + /** Called after the queued turn owns the reply lane, before model/tool execution. */ + onAdmitted?: () => void; onComplete?: () => void; }; diff --git a/src/auto-reply/reply/dispatch-from-config.test.ts b/src/auto-reply/reply/dispatch-from-config.test.ts index 826ad8a4b8d..d7cc59db664 100644 --- a/src/auto-reply/reply/dispatch-from-config.test.ts +++ b/src/auto-reply/reply/dispatch-from-config.test.ts @@ -2198,6 +2198,48 @@ describe("dispatchReplyFromConfig", () => { } }); + it("lets Gateway-owned turns reach queue resolution while a reply operation is active", async () => { + setNoAbort(); + const sessionKey = "agent:main:main"; + const activeOperation = createReplyOperation({ + sessionKey, + sessionId: "active-session", + resetTriggered: false, + }); + activeOperation.setPhase("running"); + const dispatcher = createDispatcher(); + const replyResolver = vi.fn(async () => undefined); + + try { + const result = await dispatchReplyFromConfig({ + ctx: buildTestCtx({ + Provider: "webchat", + Surface: "webchat", + SessionKey: sessionKey, + BodyForAgent: "queue this turn", + }), + cfg: emptyConfig, + dispatcher, + replyOptions: { + queuedFollowupLifecycle: { + onEnqueued: vi.fn(), + onComplete: vi.fn(), + }, + }, + replyResolver, + }); + + expect(result).toMatchObject({ + queuedFinal: false, + counts: { tool: 0, block: 0, final: 0 }, + }); + expect(replyResolver).toHaveBeenCalledTimes(1); + expect(replyRunRegistry.get(sessionKey)).toBe(activeOperation); + } finally { + activeOperation.complete(); + } + }); + it("clears stale active reply operations for terminal sessions and retries admission", async () => { setNoAbort(); const sessionKey = "agent:main:telegram:group:-1003774691294"; diff --git a/src/auto-reply/reply/dispatch-from-config.ts b/src/auto-reply/reply/dispatch-from-config.ts index d5e8773da2f..abd7b5232f3 100644 --- a/src/auto-reply/reply/dispatch-from-config.ts +++ b/src/auto-reply/reply/dispatch-from-config.ts @@ -62,11 +62,11 @@ import { toPluginMessageContext, toPluginMessageReceivedEvent, } from "../../hooks/message-hook-mappers.js"; +import { isAbortError } from "../../infra/abort-signal.js"; import { isDiagnosticsEnabled } from "../../infra/diagnostic-events.js"; import { measureDiagnosticsTimelineSpan } from "../../infra/diagnostics-timeline.js"; import { formatErrorMessage } from "../../infra/errors.js"; import { getSessionBindingService } from "../../infra/outbound/session-binding-service.js"; -import { isAbortError } from "../../infra/abort-signal.js"; import type { StuckSessionRecoveryOutcome } from "../../logging/diagnostic-session-recovery.js"; import { logMessageDispatchCompleted, @@ -1452,6 +1452,16 @@ export async function dispatchReplyFromConfig( crypto.randomUUID(); const replyTurnKind = resolveReplyTurnKind(params.replyOptions); const allowActivePreDispatch = phase === "pre_dispatch" && replyTurnKind === "visible"; + const allowGatewayQueueResolution = + phase === "dispatch" && + replyTurnKind === "visible" && + params.replyOptions?.queuedFollowupLifecycle !== undefined && + replyRunRegistry.get(dispatchOperationSessionKey) !== undefined; + if (allowGatewayQueueResolution) { + // Gateway turns need to reach getReplyFromConfig while the owner is active; + // that layer applies the session's steer/followup/collect/drop policy. + return { status: "ready" }; + } const allowSlackRoutedThreadBypass = phase === "dispatch" && shouldLetSlackRoutedThreadBypassBusyReplyOperation({ diff --git a/src/auto-reply/reply/followup-runner.test.ts b/src/auto-reply/reply/followup-runner.test.ts index 797df896e29..8edec0878e8 100644 --- a/src/auto-reply/reply/followup-runner.test.ts +++ b/src/auto-reply/reply/followup-runner.test.ts @@ -618,6 +618,32 @@ function createQueuedRun( } describe("createFollowupRunner reply-lane admission", () => { + it("notifies queued owners after admission and before model execution", async () => { + const events: string[] = []; + runEmbeddedAgentMock.mockImplementationOnce(async () => { + events.push("run"); + return { payloads: [], meta: {} }; + }); + const runner = createFollowupRunner({ + typing: createMockTypingController(), + typingMode: "instant", + sessionKey: "main", + defaultModel: "anthropic/claude", + }); + + await runner( + createQueuedRun({ + queuedLifecycle: { + onAdmitted: () => events.push("admitted"), + onComplete: () => events.push("complete"), + }, + run: { provider: "anthropic", model: "claude" }, + }), + ); + + expect(events).toEqual(["admitted", "run", "complete"]); + }); + it("passes prepared media user turns to embedded runtime dispatch", async () => { const preparedUserTurnMessage = { role: "user", @@ -888,6 +914,7 @@ describe("createFollowupRunner reply-lane admission", () => { it("preserves non-compaction preflight failures for queued followup runs", async () => { runPreflightCompactionIfNeededMock.mockRejectedValueOnce(new Error("session load failed")); + const onComplete = vi.fn(); const runner = createFollowupRunner({ typing: createMockTypingController(), typingMode: "instant", @@ -906,12 +933,14 @@ describe("createFollowupRunner reply-lane admission", () => { model: "claude", sessionKey: "main", }, + queuedLifecycle: { onComplete }, }), ), ).rejects.toThrow("session load failed"); expect(runEmbeddedAgentMock).not.toHaveBeenCalled(); expect(routeReplyMock).not.toHaveBeenCalled(); + expect(onComplete).not.toHaveBeenCalled(); }); }); diff --git a/src/auto-reply/reply/followup-runner.ts b/src/auto-reply/reply/followup-runner.ts index 008fc67af94..daa17572d56 100644 --- a/src/auto-reply/reply/followup-runner.ts +++ b/src/auto-reply/reply/followup-runner.ts @@ -521,6 +521,7 @@ export function createFollowupRunner(params: { const queuedImageOrder = queued.imageOrder ?? opts?.imageOrder; let replyOperation: ReplyOperation | undefined; let deferred = false; + let failed = false; try { queued.run.config = await resolveQueuedReplyExecutionConfig(queued.run.config, { @@ -626,6 +627,9 @@ export function createFollowupRunner(params: { return; } replyOperation = admission.operation; + // Multi-source collected turns become atomic at reply-lane admission. + // Their queue owner uses this boundary to retire source cancellation ids. + effectiveQueued.queuedLifecycle?.onAdmitted?.(); if (replyOperation.sessionId !== run.sessionId) { run = { ...run, sessionId: replyOperation.sessionId }; effectiveQueued = { ...effectiveQueued, run }; @@ -1579,6 +1583,9 @@ export function createFollowupRunner(params: { }, { runId }, ); + } catch (err) { + failed = true; + throw err; } finally { for (const end of endDeliveryCorrelations.toReversed()) { try { @@ -1589,7 +1596,9 @@ export function createFollowupRunner(params: { ); } } - if (!deferred) { + // A thrown attempt stays in the drain queue for retry. Its lifecycle + // identity remains live until the drain later consumes or drops it. + if (!deferred && !failed) { completeFollowupRunLifecycle(queued); } replyOperation?.complete(); diff --git a/src/auto-reply/reply/get-reply-run.ts b/src/auto-reply/reply/get-reply-run.ts index 8055ed3be45..cdfc5d0b5fa 100644 --- a/src/auto-reply/reply/get-reply-run.ts +++ b/src/auto-reply/reply/get-reply-run.ts @@ -1267,8 +1267,15 @@ export async function runPreparedReply( extractedFileImages: opts?.extractedFileImages, }), ); + // Abort-signal attachment for queued followups: + // - room_event: always inherit (source admission fence / ambient cancel). + // - Gateway-owned lifecycle (chat.send): always inherit so Esc can cancel a + // turn after chat.send terminalizes while still queued. + // - plain user_request without lifecycle: deliberately detach from the + // source/active-lane signal so a superseded parent abort does not cancel a + // still-valid queued user turn. const queuedFollowupAbortSignal = - inboundEventKind === "room_event" + opts?.queuedFollowupLifecycle || inboundEventKind === "room_event" ? (opts?.queuedFollowupAbortSignal ?? opts?.abortSignal) : undefined; const userTurnMediaForPersistence = buildPersistedUserTurnMediaInputsFromFields(ctx); diff --git a/src/auto-reply/reply/queue.collect.test.ts b/src/auto-reply/reply/queue.collect.test.ts index eb7eb182720..2df0e108a27 100644 --- a/src/auto-reply/reply/queue.collect.test.ts +++ b/src/auto-reply/reply/queue.collect.test.ts @@ -3,6 +3,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { describe, expect, it, vi } from "vitest"; +import { createUserTurnTranscriptRecorder } from "../../sessions/user-turn-transcript.js"; import type { FollowupRun, QueueSettings } from "./queue.js"; import { clearFollowupQueue, @@ -22,6 +23,25 @@ import { getExistingFollowupQueue } from "./queue/state.js"; installQueueRuntimeErrorSilencer(); describe("followup queue collect routing", () => { + it("does not enqueue when the external lifecycle rejects the run identity", () => { + const key = `test-rejected-lifecycle-${Date.now()}`; + const onEnqueued = vi.fn(() => false); + const run = createRun({ prompt: "duplicate owner" }); + run.queuedLifecycle = { onEnqueued }; + + const enqueued = enqueueFollowupRun(key, run, { + mode: "followup", + debounceMs: 10_000, + cap: 50, + dropPolicy: "summarize", + }); + + expect(enqueued).toBe(false); + expect(onEnqueued).toHaveBeenCalledTimes(1); + expect(getExistingFollowupQueue(key)?.items).toEqual([]); + clearFollowupQueue(key); + }); + it("does not collect when destinations differ", async () => { const key = `test-collect-diff-to-${Date.now()}`; const calls: FollowupRun[] = []; @@ -509,7 +529,7 @@ describe("followup queue collect routing", () => { const queue = getExistingFollowupQueue(key); expect(accepted).toEqual([true, true, true, true, true, true, true]); expect(queue?.summaryElisions).toHaveLength(2); - expect(queue?.summaryElisions.map((entry) => entry.source.originatingTo)).toEqual([ + expect(queue?.summaryElisions.map((entry) => entry.sources.at(-1)?.originatingTo)).toEqual([ "channel:B", "channel:A", ]); @@ -521,6 +541,73 @@ describe("followup queue collect routing", () => { clearFollowupQueue(key); }); + it("bounds retained overflow cancellation identities by the item cap", () => { + const key = `test-collect-overflow-source-bound-${Date.now()}`; + const completions = Array.from({ length: 8 }, () => vi.fn()); + const settings: QueueSettings = { + mode: "collect", + debounceMs: 0, + cap: 2, + dropPolicy: "summarize", + }; + + for (const [index, onComplete] of completions.entries()) { + enqueueFollowupRun( + key, + { + ...createRun({ + prompt: `message ${index}`, + originatingChannel: "slack", + originatingTo: "channel:A", + originatingChatType: "channel", + }), + queuedLifecycle: { onComplete }, + }, + settings, + ); + } + + const queue = getExistingFollowupQueue(key); + expect(queue?.summaryElisions.flatMap((entry) => entry.sources)).toHaveLength(2); + expect( + queue?.summaryElisions.flatMap((entry) => entry.sources.map((source) => source.prompt)), + ).toEqual(["message 2", "message 3"]); + expect(queue?.evictedSummaryCount).toBe(2); + expect(completions.map((onComplete) => onComplete.mock.calls.length)).toEqual([ + 1, 1, 0, 0, 0, 0, 0, 0, + ]); + clearFollowupQueue(key); + }); + + it("does not register a drop:new source that the full queue rejects", () => { + const key = `test-drop-new-lifecycle-${Date.now()}`; + const onEnqueued = vi.fn(); + const onComplete = vi.fn(); + const settings: QueueSettings = { + mode: "followup", + debounceMs: 0, + cap: 1, + dropPolicy: "new", + }; + + expect(enqueueFollowupRun(key, createRun({ prompt: "existing" }), settings)).toBe(true); + expect( + enqueueFollowupRun( + key, + { + ...createRun({ prompt: "rejected" }), + queuedLifecycle: { onEnqueued, onComplete }, + }, + settings, + ), + ).toBe(false); + + expect(onEnqueued).not.toHaveBeenCalled(); + expect(onComplete).toHaveBeenCalledOnce(); + expect(getExistingFollowupQueue(key)?.items.map((item) => item.prompt)).toEqual(["existing"]); + clearFollowupQueue(key); + }); + it("keeps retained excess contexts isolated after evicting the oldest metadata", async () => { const key = `test-collect-overflow-evicted-context-${Date.now()}`; const calls: FollowupRun[] = []; @@ -875,7 +962,7 @@ describe("followup queue collect routing", () => { }); await done.promise; - expect(calls[0]?.prompt).toContain("Dropped 2 messages"); + expect(calls[0]?.prompt).toContain("Dropped 1 message"); expect(calls[0]?.run.model).toBe("model-b"); expect(calls[0]?.run.authProfileId).toBe("auth-b"); expect(calls[1]?.prompt).toContain("- retained"); @@ -1100,7 +1187,7 @@ describe("followup queue collect routing", () => { }, ); - it("uses current run settings without dropped runtime context for split summaries", async () => { + it("drops an aborted split summary before running the surviving item", async () => { const key = `test-collect-overflow-current-run-${Date.now()}`; const calls: FollowupRun[] = []; const done = createDeferred(); @@ -1160,18 +1247,15 @@ describe("followup queue collect routing", () => { scheduleFollowupDrain(key, async (run) => { calls.push(run); - if (calls.length >= 2) { - done.resolve(); - } + done.resolve(); }); await done.promise; + expect(calls).toHaveLength(1); expect(calls[0]?.run.model).toBe("current-model"); - expect(calls[0]?.abortSignal).toBeUndefined(); - expect(calls[0]?.currentInboundContext).toBeUndefined(); - expect(calls[0]?.originatingChatType).toBe("direct"); - expect(calls[0]?.run.senderId).toBe("guest"); - expect(calls[0]?.run.senderIsOwner).toBe(false); + expect(calls[0]?.originatingChatType).toBe("channel"); + expect(calls[0]?.run.senderId).toBe("owner"); + expect(calls[0]?.run.senderIsOwner).toBe(true); }); it("removes a delivered split summary by source identity after concurrent enqueue", async () => { @@ -1755,6 +1839,45 @@ describe("followup queue collect routing", () => { expect(calls[1]?.prompt).toContain("(from Owner)"); }); + it("splits collect batches when queued cancellation owners differ", async () => { + const key = `test-collect-cancel-owner-split-${Date.now()}`; + const calls: FollowupRun[] = []; + const done = createDeferred(); + const settings: QueueSettings = { mode: "collect", debounceMs: 0 }; + + for (const [prompt, ownerKey] of [ + ["first", "connection:one"], + ["second", "connection:two"], + ] as const) { + enqueueFollowupRun( + key, + { + ...createRun({ + prompt, + originatingChannel: "webchat", + originatingTo: "session:main", + }), + queuedLifecycle: { ownerKey }, + }, + settings, + ); + } + + scheduleFollowupDrain(key, async (run) => { + calls.push(run); + if (calls.length === 2) { + done.resolve(); + } + }); + await done.promise; + + expect(calls).toHaveLength(2); + expect(calls[0]?.prompt).toContain("first"); + expect(calls[0]?.prompt).not.toContain("second"); + expect(calls[1]?.prompt).toContain("second"); + expect(calls[1]?.prompt).not.toContain("first"); + }); + it("keeps one collect batch when authorization context matches", async () => { const key = `test-collect-auth-match-${Date.now()}`; const calls: FollowupRun[] = []; @@ -1816,6 +1939,7 @@ describe("followup queue collect routing", () => { expect(calls).toHaveLength(1); expect(calls[0]?.prompt).toContain("first"); expect(calls[0]?.prompt).toContain("second"); + expect(calls[0]?.abortSignal).toBeUndefined(); expect(calls[0]?.prompt).toContain("(from Guest)"); }); @@ -2211,6 +2335,25 @@ describe("followup queue collect routing", () => { expect(calls[0]?.prompt).toContain("two"); }); + it("collects matching local webchat routes without an external destination", async () => { + const key = `test-collect-local-webchat-${Date.now()}`; + const calls: FollowupRun[] = []; + const done = createDeferred(); + const settings: QueueSettings = { mode: "collect", debounceMs: 0 }; + + enqueueFollowupRun(key, createRun({ prompt: "one", originatingChannel: "webchat" }), settings); + enqueueFollowupRun(key, createRun({ prompt: "two", originatingChannel: "webchat" }), settings); + scheduleFollowupDrain(key, async (run) => { + calls.push(run); + done.resolve(); + }); + await done.promise; + + expect(calls).toHaveLength(1); + expect(calls[0]?.prompt).toContain("one"); + expect(calls[0]?.prompt).toContain("two"); + }); + it("does not collect Slack messages when thread ids differ", async () => { const key = `test-collect-slack-thread-diff-${Date.now()}`; const calls: FollowupRun[] = []; @@ -2451,7 +2594,7 @@ describe("followup queue collect routing", () => { } }); - it("clears overflow summaries when aborts empty the queue", async () => { + it("keeps overflow summaries when aborts remove only the live item", async () => { const key = `test-overflow-summary-aborted-${Date.now()}`; const calls: FollowupRun[] = []; const cleaned: FollowupRun[] = []; @@ -2487,7 +2630,8 @@ describe("followup queue collect routing", () => { setTimeout(resolve, 10); }); - expect(calls).toHaveLength(0); + expect(calls).toHaveLength(1); + expect(calls[0]?.prompt).toContain("- dropped"); expect(cleaned.map((run) => run.prompt)).toEqual(["aborted"]); expect(onComplete).toHaveBeenCalledTimes(1); expect(getExistingFollowupQueue(key)).toBeUndefined(); @@ -2824,7 +2968,7 @@ describe("followup queue collect routing", () => { expect(calls[1]?.prompt).toBe("live followup"); }); - it("keeps summarized room-event lifecycle until the overflow summary drains", async () => { + it("drops an aborted summarized room event before overflow delivery", async () => { const key = `test-overflow-summary-lifecycle-${Date.now()}`; const calls: FollowupRun[] = []; const done = createDeferred(); @@ -2832,9 +2976,7 @@ describe("followup queue collect routing", () => { const onComplete = vi.fn(); const runFollowup = async (run: FollowupRun) => { calls.push(run); - if (calls.length >= 2) { - done.resolve(); - } + done.resolve(); }; const settings: QueueSettings = { mode: "followup", @@ -2862,15 +3004,62 @@ describe("followup queue collect routing", () => { scheduleFollowupDrain(key, runFollowup); await done.promise; - expect(calls).toHaveLength(2); - expect(calls[0]?.prompt).toContain("[Queue overflow] Dropped 1 message due to cap."); - expect(calls[0]?.currentInboundEventKind).toBe("room_event"); - expect(calls[0]?.currentInboundContext).toBeUndefined(); - expect(calls[0]?.abortSignal).toBeUndefined(); - expect(calls[1]?.prompt).toBe("live followup"); + expect(calls).toHaveLength(1); + expect(calls[0]?.prompt).toBe("live followup"); expect(onComplete).toHaveBeenCalledTimes(1); }); + it("retains summarized source identities through admitted overflow delivery", async () => { + const key = `test-overflow-summary-admitted-lifecycle-${Date.now()}`; + const calls: FollowupRun[] = []; + const done = createDeferred(); + const sourceCompletions = [vi.fn(), vi.fn()]; + const sourceCancellationRetirements = [vi.fn(), vi.fn()]; + const settings: QueueSettings = { + mode: "followup", + debounceMs: 0, + cap: 1, + dropPolicy: "summarize", + }; + + for (const [index, prompt] of ["first dropped", "second dropped"].entries()) { + enqueueFollowupRun( + key, + { + ...createRun({ prompt }), + abortSignal: new AbortController().signal, + queuedLifecycle: { + onCancellationRetired: sourceCancellationRetirements[index], + onComplete: sourceCompletions[index], + }, + }, + settings, + ); + } + enqueueFollowupRun(key, createRun({ prompt: "live followup" }), settings); + + scheduleFollowupDrain(key, async (run) => { + calls.push(run); + if (calls.length === 1) { + expect(run.prompt).toContain("[Queue overflow] Dropped 2 messages due to cap."); + run.queuedLifecycle?.onAdmitted?.(); + expect(sourceCancellationRetirements[0]).toHaveBeenCalledTimes(1); + expect(sourceCancellationRetirements[1]).not.toHaveBeenCalled(); + expect(sourceCompletions[0]).not.toHaveBeenCalled(); + expect(sourceCompletions[1]).not.toHaveBeenCalled(); + run.queuedLifecycle?.onComplete?.(); + expect(sourceCompletions[0]).toHaveBeenCalledTimes(1); + expect(sourceCompletions[1]).toHaveBeenCalledTimes(1); + return; + } + done.resolve(); + }); + await done.promise; + + expect(calls).toHaveLength(2); + expect(calls[1]?.prompt).toBe("live followup"); + }); + it("completes summarized room-event lifecycle when overflow summary delivery fails", async () => { const key = `test-overflow-summary-lifecycle-failure-${Date.now()}`; const calls: FollowupRun[] = []; @@ -2914,14 +3103,15 @@ describe("followup queue collect routing", () => { setTimeout(resolve, 10); }); - expect(onComplete).toHaveBeenCalledTimes(1); + expect(onComplete).not.toHaveBeenCalled(); expect(getExistingFollowupQueue(key)?.summarySources).toHaveLength(1); expect(getExistingFollowupQueue(key)?.summarySources[0]?.currentInboundEventKind).toBe( "room_event", ); - expect(getExistingFollowupQueue(key)?.summarySources[0]?.queuedLifecycle).toBeUndefined(); + expect(getExistingFollowupQueue(key)?.summarySources[0]?.queuedLifecycle).toBeDefined(); expect(getExistingFollowupQueue(key)?.summarySources[0]?.currentInboundContext).toBeUndefined(); + scheduleFollowupDrain(key, runFollowup); releaseRetry.resolve(); await done.promise; @@ -2929,8 +3119,436 @@ describe("followup queue collect routing", () => { expect(calls[1]?.prompt).toContain("[Queue overflow] Dropped 1 message due to cap."); expect(calls[1]?.prompt).toContain("- dropped ambient"); expect(calls[1]?.currentInboundEventKind).toBe("room_event"); + await vi.waitFor(() => expect(onComplete).toHaveBeenCalledTimes(1)); expect(onComplete).toHaveBeenCalledTimes(1); }); + + it("collects compatible cancelable turns and completes each source lifecycle", async () => { + const key = `test-collect-cancelable-${Date.now()}`; + const calls: FollowupRun[] = []; + const done = createDeferred(); + const firstComplete = vi.fn(); + const secondComplete = vi.fn(); + const runFollowup = async (run: FollowupRun) => { + calls.push(run); + done.resolve(); + }; + const settings: QueueSettings = { mode: "collect", debounceMs: 0 }; + + enqueueFollowupRun( + key, + { + ...createRun({ prompt: "first" }), + abortSignal: new AbortController().signal, + queuedLifecycle: { onComplete: firstComplete }, + }, + settings, + ); + enqueueFollowupRun( + key, + { + ...createRun({ prompt: "second" }), + abortSignal: new AbortController().signal, + queuedLifecycle: { onComplete: secondComplete }, + }, + settings, + ); + + scheduleFollowupDrain(key, runFollowup); + await done.promise; + + expect(calls).toHaveLength(1); + expect(calls[0]?.prompt).toContain("first"); + expect(calls[0]?.prompt).toContain("second"); + await vi.waitFor(() => expect(firstComplete).toHaveBeenCalledTimes(1)); + expect(firstComplete).toHaveBeenCalledTimes(1); + expect(secondComplete).toHaveBeenCalledTimes(1); + }); + + it("collects transcript-owned turns under one aggregate recorder", async () => { + const key = `test-collect-transcript-owner-${Date.now()}`; + const calls: FollowupRun[] = []; + const done = createDeferred(); + const firstComplete = vi.fn(); + const secondComplete = vi.fn(); + const firstCorrelation = { begin: vi.fn() }; + const secondCorrelation = { begin: vi.fn() }; + const createRecorder = (text: string, mediaPath: string) => + createUserTurnTranscriptRecorder({ + input: { text, media: [{ path: mediaPath, contentType: "image/png" }] }, + target: { transcriptPath: "/tmp/session.jsonl" }, + updateMode: "none", + }); + const firstRecorder = createRecorder("first transcript", "/tmp/first.png"); + const secondRecorder = createRecorder("second transcript", "/tmp/second.png"); + const settings: QueueSettings = { mode: "collect", debounceMs: 0 }; + + for (const [prompt, recorder, onComplete, deliveryCorrelation] of [ + ["first", firstRecorder, firstComplete, firstCorrelation], + ["second", secondRecorder, secondComplete, secondCorrelation], + ] as const) { + enqueueFollowupRun( + key, + { + ...createRun({ prompt }), + transcriptPrompt: `${prompt} transcript`, + userTurnTranscriptRecorder: recorder, + currentInboundContext: { text: "shared gateway context", promptJoiner: " " }, + deliveryCorrelations: [deliveryCorrelation], + abortSignal: new AbortController().signal, + queuedLifecycle: { onComplete }, + }, + settings, + ); + } + + scheduleFollowupDrain(key, async (run) => { + calls.push(run); + done.resolve(); + }); + await done.promise; + + expect(calls).toHaveLength(1); + expect(calls[0]?.prompt).toContain("first"); + expect(calls[0]?.prompt).toContain("second"); + expect(calls[0]?.transcriptPrompt).toContain("first transcript"); + expect(calls[0]?.transcriptPrompt).toContain("second transcript"); + expect(calls[0]?.currentInboundContext?.text).toContain( + "Queued #1 context:\nshared gateway context", + ); + expect(calls[0]?.currentInboundContext?.text).toContain( + "Queued #2 context:\nshared gateway context", + ); + expect(calls[0]?.currentInboundContext?.promptJoiner).toBe("\n\n"); + expect(calls[0]?.deliveryCorrelations).toEqual([firstCorrelation, secondCorrelation]); + expect(calls[0]?.userTurnTranscriptRecorder).not.toBe(firstRecorder); + expect(calls[0]?.userTurnTranscriptRecorder).not.toBe(secondRecorder); + const message = await calls[0]?.userTurnTranscriptRecorder?.resolveMessage(); + expect(message?.content).toContain("first transcript"); + expect(message?.content).toContain("second transcript"); + expect((message as unknown as { MediaPaths?: string[] } | undefined)?.MediaPaths).toEqual([ + "/tmp/first.png", + "/tmp/second.png", + ]); + await vi.waitFor(() => expect(firstComplete).toHaveBeenCalledTimes(1)); + expect(secondComplete).toHaveBeenCalledTimes(1); + }); + + it("pairs differing inbound runtime contexts inside one collected turn", async () => { + const key = `test-collect-runtime-context-split-${Date.now()}`; + const calls: FollowupRun[] = []; + const done = createDeferred(); + const settings: QueueSettings = { mode: "collect", debounceMs: 0 }; + + for (const [prompt, contextText] of [ + ["first", "context one"], + ["second", "context two"], + ] as const) { + enqueueFollowupRun( + key, + { + ...createRun({ prompt }), + currentInboundContext: { text: contextText }, + }, + settings, + ); + } + + scheduleFollowupDrain(key, async (run) => { + calls.push(run); + done.resolve(); + }); + await done.promise; + + expect(calls).toHaveLength(1); + expect(calls[0]?.prompt).toContain("first"); + expect(calls[0]?.prompt).toContain("second"); + expect(calls[0]?.currentInboundContext?.text).toContain("Queued #1 context:\ncontext one"); + expect(calls[0]?.currentInboundContext?.text).toContain("Queued #2 context:\ncontext two"); + }); + + it("does not let one source cancel an admitted collected run", async () => { + const key = `test-collect-transcript-cancel-${Date.now()}`; + const calls: FollowupRun[] = []; + const done = createDeferred(); + const canceled = new AbortController(); + const survivor = new AbortController(); + const sourceCompletions = [vi.fn(), vi.fn()]; + const sourceCancellationRetirements = [vi.fn(), vi.fn()]; + let firstResolvedContent = ""; + const settings: QueueSettings = { mode: "collect", debounceMs: 0 }; + const createRecorder = (text: string) => + createUserTurnTranscriptRecorder({ + input: { text }, + target: { transcriptPath: "/tmp/session.jsonl" }, + updateMode: "none", + }); + + for (const [index, [prompt, abortSignal]] of ( + [ + ["canceled", canceled.signal], + ["survivor", survivor.signal], + ] as const + ).entries()) { + enqueueFollowupRun( + key, + { + ...createRun({ prompt }), + transcriptPrompt: `${prompt} transcript`, + userTurnTranscriptRecorder: createRecorder(`${prompt} transcript`), + abortSignal, + queuedLifecycle: { + onCancellationRetired: sourceCancellationRetirements[index], + onComplete: sourceCompletions[index], + }, + }, + settings, + ); + } + + scheduleFollowupDrain(key, async (run) => { + calls.push(run); + if (calls.length === 1) { + expect(run.abortSignal).toBeDefined(); + expect(run.abortSignal).not.toBe(survivor.signal); + run.queuedLifecycle?.onAdmitted?.(); + expect(sourceCancellationRetirements[0]).toHaveBeenCalledTimes(1); + expect(sourceCancellationRetirements[1]).not.toHaveBeenCalled(); + expect(sourceCompletions[0]).not.toHaveBeenCalled(); + expect(sourceCompletions[1]).not.toHaveBeenCalled(); + canceled.abort(); + expect(run.abortSignal?.aborted).toBe(false); + const resolved = await run.userTurnTranscriptRecorder?.resolveMessage(); + firstResolvedContent = typeof resolved?.content === "string" ? resolved.content : ""; + done.resolve(); + } + }); + await done.promise; + + expect(calls).toHaveLength(1); + expect(firstResolvedContent).toContain("survivor transcript"); + expect(firstResolvedContent).toContain("canceled transcript"); + await vi.waitFor(() => expect(sourceCompletions[1]).toHaveBeenCalledTimes(1)); + expect(sourceCompletions[0]).toHaveBeenCalledTimes(1); + expect(sourceCompletions[1]).toHaveBeenCalledTimes(1); + expect(getExistingFollowupQueue(key)?.items ?? []).toHaveLength(0); + }); + + it("keeps queue cancellation connected after collect admission", async () => { + const key = `test-collect-queue-cancel-${Date.now()}`; + const done = createDeferred(); + const settings: QueueSettings = { mode: "collect", debounceMs: 0 }; + + enqueueFollowupRun(key, createRun({ prompt: "first" }), settings); + enqueueFollowupRun(key, createRun({ prompt: "second" }), settings); + + scheduleFollowupDrain(key, async (run) => { + expect(run.abortSignal).toBeUndefined(); + expect(run.queueAbortSignal?.aborted).toBe(false); + run.queuedLifecycle?.onAdmitted?.(); + clearFollowupQueue(key); + expect(run.queueAbortSignal?.aborted).toBe(true); + done.resolve(); + }); + await done.promise; + }); + + it.each(["first", "last"] as const)( + "retries survivors when the %s source owns the sole pre-admission cancel signal", + async (canceledPosition) => { + const key = `test-collect-pre-admission-cancel-${canceledPosition}-${Date.now()}`; + const canceled = new AbortController(); + const canceledComplete = vi.fn(); + const survivorComplete = vi.fn(); + const calls: FollowupRun[] = []; + const done = createDeferred(); + const settings: QueueSettings = { mode: "collect", debounceMs: 0 }; + + const enqueueSource = (prompt: string, onComplete: () => void, abortSignal?: AbortSignal) => { + const source: FollowupRun = { + ...createRun({ prompt }), + queuedLifecycle: { onComplete }, + }; + if (abortSignal) { + source.abortSignal = abortSignal; + } + enqueueFollowupRun(key, source, settings); + }; + if (canceledPosition === "first") { + enqueueSource("canceled", canceledComplete, canceled.signal); + enqueueSource("survivor", survivorComplete); + } else { + enqueueSource("survivor", survivorComplete); + enqueueSource("canceled", canceledComplete, canceled.signal); + } + + scheduleFollowupDrain(key, async (run) => { + calls.push(run); + if (calls.length === 1) { + canceled.abort(); + expect(run.abortSignal?.aborted).toBe(true); + return; + } + done.resolve(); + }); + await done.promise; + + expect(calls).toHaveLength(2); + expect(calls[0]?.prompt).toContain("canceled"); + expect(calls[0]?.prompt).toContain("survivor"); + expect(calls[1]?.prompt).toContain("survivor"); + expect(calls[1]?.prompt).not.toContain("canceled"); + await vi.waitFor(() => expect(survivorComplete).toHaveBeenCalledTimes(1)); + expect(canceledComplete).toHaveBeenCalledTimes(1); + }, + ); + + it("keeps summarized work when a different cancelable live item is aborted", async () => { + const key = `test-summary-owner-isolation-${Date.now()}`; + const calls: FollowupRun[] = []; + const done = createDeferred(); + const summarizedComplete = vi.fn(); + const abortedComplete = vi.fn(); + const aborted = new AbortController(); + const runFollowup = async (run: FollowupRun) => { + if (run.abortSignal?.aborted) { + return; + } + calls.push(run); + done.resolve(); + }; + const settings: QueueSettings = { + mode: "followup", + debounceMs: 0, + cap: 1, + dropPolicy: "summarize", + }; + + enqueueFollowupRun( + key, + { + ...createRun({ prompt: "owner A summary" }), + abortSignal: new AbortController().signal, + queuedLifecycle: { onComplete: summarizedComplete }, + }, + settings, + ); + enqueueFollowupRun( + key, + { + ...createRun({ prompt: "owner B live" }), + abortSignal: aborted.signal, + queuedLifecycle: { onComplete: abortedComplete }, + }, + settings, + ); + aborted.abort(); + + scheduleFollowupDrain(key, runFollowup); + await done.promise; + + expect(calls).toHaveLength(1); + expect(calls[0]?.prompt).toContain("owner A summary"); + expect(calls[0]?.prompt).not.toContain("owner B live"); + await vi.waitFor(() => expect(summarizedComplete).toHaveBeenCalledTimes(1)); + expect(summarizedComplete).toHaveBeenCalledTimes(1); + expect(abortedComplete).toHaveBeenCalledTimes(1); + }); + + it("removes an aborted elided source without leaking it into the summary", async () => { + const key = `test-elided-summary-cancel-${Date.now()}`; + const calls: FollowupRun[] = []; + const done = createDeferred(); + const elidedComplete = vi.fn(); + const elided = new AbortController(); + const runFollowup = async (run: FollowupRun) => { + if (run.abortSignal?.aborted) { + return; + } + calls.push(run); + if (calls.length === 2) { + done.resolve(); + } + }; + const settings: QueueSettings = { + mode: "followup", + debounceMs: 0, + cap: 1, + dropPolicy: "summarize", + }; + + enqueueFollowupRun( + key, + { + ...createRun({ prompt: "elided and cancelled" }), + abortSignal: elided.signal, + queuedLifecycle: { onComplete: elidedComplete }, + }, + settings, + ); + enqueueFollowupRun(key, createRun({ prompt: "retained summary" }), settings); + enqueueFollowupRun(key, createRun({ prompt: "live item" }), settings); + elided.abort(); + + scheduleFollowupDrain(key, runFollowup); + await done.promise; + + expect(calls.map((call) => call.prompt).join("\n")).not.toContain("elided and cancelled"); + expect(calls[0]?.prompt).toContain("retained summary"); + expect(calls[1]?.prompt).toBe("live item"); + expect(elidedComplete).toHaveBeenCalledTimes(1); + }); + + it("does not replay elided sources after an admitted summary failure", async () => { + const key = `test-elided-summary-admitted-failure-${Date.now()}`; + const calls: FollowupRun[] = []; + const done = createDeferred(); + const elidedComplete = vi.fn(); + const retainedComplete = vi.fn(); + const settings: QueueSettings = { + mode: "followup", + debounceMs: 0, + cap: 1, + dropPolicy: "summarize", + }; + + enqueueFollowupRun( + key, + { + ...createRun({ prompt: "elided source" }), + queuedLifecycle: { onComplete: elidedComplete }, + }, + settings, + ); + enqueueFollowupRun( + key, + { + ...createRun({ prompt: "retained source" }), + queuedLifecycle: { onComplete: retainedComplete }, + }, + settings, + ); + enqueueFollowupRun(key, createRun({ prompt: "live item" }), settings); + + scheduleFollowupDrain(key, async (run) => { + calls.push(run); + if (calls.length === 1) { + expect(run.prompt).toContain("Dropped 2 messages"); + expect(run.prompt).toContain("retained source"); + run.queuedLifecycle?.onAdmitted?.(); + expect(getExistingFollowupQueue(key)?.summaryElisions).toEqual([]); + expect(getExistingFollowupQueue(key)?.droppedCount).toBe(0); + throw new Error("admitted summary failure"); + } + done.resolve(); + }); + await done.promise; + + expect(calls).toHaveLength(2); + expect(calls[1]?.prompt).toBe("live item"); + expect(elidedComplete).toHaveBeenCalledOnce(); + expect(retainedComplete).toHaveBeenCalledOnce(); + }); }); describe("resolveFollowupAuthorizationKey", () => { @@ -2969,6 +3587,21 @@ describe("resolveFollowupAuthorizationKey", () => { ); }); + it("changes when the approval reviewer device changes", () => { + const run = createRun({ prompt: "one" }).run; + expect( + resolveFollowupAuthorizationKey({ + ...run, + approvalReviewerDeviceId: "device-a", + }), + ).not.toBe( + resolveFollowupAuthorizationKey({ + ...run, + approvalReviewerDeviceId: "device-b", + }), + ); + }); + it("does not change when only sender display fields change", () => { const run = createRun({ prompt: "one" }).run; expect( diff --git a/src/auto-reply/reply/queue.drain-restart.test.ts b/src/auto-reply/reply/queue.drain-restart.test.ts index aae0568f921..9c62527aefb 100644 --- a/src/auto-reply/reply/queue.drain-restart.test.ts +++ b/src/auto-reply/reply/queue.drain-restart.test.ts @@ -2,12 +2,18 @@ import { importFreshModule } from "openclaw/plugin-sdk/test-fixtures"; import { describe, expect, it, vi } from "vitest"; import type { FollowupRun, QueueSettings } from "./queue.js"; -import { enqueueFollowupRun, FollowupRunDeferredError, scheduleFollowupDrain } from "./queue.js"; +import { + clearFollowupQueue, + enqueueFollowupRun, + FollowupRunDeferredError, + scheduleFollowupDrain, +} from "./queue.js"; import { createDeferred, createQueueTestRun as createRun, installQueueRuntimeErrorSilencer, } from "./queue.test-helpers.js"; +import { getExistingFollowupQueue } from "./queue/state.js"; installQueueRuntimeErrorSilencer(); @@ -376,6 +382,44 @@ describe("followup queue drain restart after idle window", () => { expect(prompts[1]).not.toContain("original dropped while busy"); }); + it("bounds overflow identities across repeated deferred retries", async () => { + const key = `test-deferred-summary-bound-${Date.now()}`; + const settings: QueueSettings = { + mode: "followup", + debounceMs: 0, + cap: 1, + dropPolicy: "summarize", + }; + const completed = createDeferred(); + let retainedIdentityCount = 0; + let attempts = 0; + + const runFollowup = async () => { + attempts += 1; + if (attempts === 3) { + const queue = getExistingFollowupQueue(key); + retainedIdentityCount = + queue?.summaryElisions.reduce((count, entry) => count + entry.sources.length, 0) ?? 0; + clearFollowupQueue(key); + completed.resolve(); + return; + } + if (attempts <= 2) { + enqueueFollowupRun(key, createRun({ prompt: `dropped on retry ${attempts}` }), settings); + enqueueFollowupRun(key, createRun({ prompt: `kept on retry ${attempts}` }), settings); + throw new FollowupRunDeferredError("reply lane busy"); + } + }; + + enqueueFollowupRun(key, createRun({ prompt: "original dropped" }), settings); + enqueueFollowupRun(key, createRun({ prompt: "original kept" }), settings); + scheduleFollowupDrain(key, runFollowup); + await completed.promise; + + expect(attempts).toBe(3); + expect(retainedIdentityCount).toBeLessThanOrEqual(2); + }); + it("does not process messages after clearSessionQueues clears the callback", async () => { const key = `test-clear-callback-${Date.now()}`; const calls: FollowupRun[] = []; diff --git a/src/auto-reply/reply/queue/drain.ts b/src/auto-reply/reply/queue/drain.ts index 3ebecfdf562..6751c87fd10 100644 --- a/src/auto-reply/reply/queue/drain.ts +++ b/src/auto-reply/reply/queue/drain.ts @@ -10,12 +10,14 @@ import { channelRouteDedupeKey, } from "../../../plugin-sdk/channel-route.js"; import { defaultRuntime } from "../../../runtime.js"; -import { createUserTurnTranscriptRecorder } from "../../../sessions/user-turn-transcript.js"; +import { + buildPersistedUserTurnMediaInputsFromFields, + createUserTurnTranscriptRecorder, +} from "../../../sessions/user-turn-transcript.js"; import { resolveGlobalMap } from "../../../shared/global-singleton.js"; import { buildCollectPrompt, beginQueueDrain, - clearQueueSummaryState, drainCollectQueueStep, drainNextQueueItem, hasCrossChannelItems, @@ -24,11 +26,12 @@ import { waitForQueueDebounce, } from "../../../utils/queue-helpers.js"; import { isRoutableChannel } from "../route-reply.js"; -import { FOLLOWUP_QUEUES } from "./state.js"; +import { FOLLOWUP_QUEUES, trimSummaryElisionsToCap } from "./state.js"; import { completeFollowupRunLifecycle, isFollowupRunAborted, isFollowupRunDeferredError, + retireFollowupRunCancellation, type FollowupRun, } from "./types.js"; @@ -120,6 +123,7 @@ export function resolveFollowupAuthorizationKey(run: FollowupRun["run"]): string run.bashElevated?.enabled === true, run.bashElevated?.allowed === true, run.bashElevated?.defaultLevel ?? "", + run.approvalReviewerDeviceId ?? "", ]); } @@ -138,6 +142,7 @@ export function resolveFollowupDeliveryContextKey(run: FollowupRun): string { run.originatingReplyToMode ?? "", normalizeChatType(run.originatingChatType) ?? "", resolveFollowupAuthorizationKey(execution), + run.queuedLifecycle?.ownerKey ?? "", normalizeOptionalString(execution.runtimePolicySessionKey ?? execution.sessionKey) ?? "", execution.messageProvider ?? "", execution.chatType ?? "", @@ -202,10 +207,14 @@ function splitCollectItemsByDeliveryContext(items: FollowupRun[]): FollowupRun[] } function renderCollectItem(item: FollowupRun, idx: number): string { + return renderCollectItemPrompt(item, idx, item.prompt); +} + +function renderCollectItemPrompt(item: FollowupRun, idx: number, prompt: string): string { const senderLabel = item.run.senderName ?? item.run.senderUsername ?? item.run.senderId ?? item.run.senderE164; const senderSuffix = senderLabel ? ` (from ${senderLabel})` : ""; - return `---\nQueued #${idx + 1}${senderSuffix}\n${item.prompt}`.trim(); + return `---\nQueued #${idx + 1}${senderSuffix}\n${prompt}`.trim(); } function collectQueuedImages(items: FollowupRun[]): Pick { @@ -245,106 +254,257 @@ function hasCurrentTurnRuntimeMetadata(item: FollowupRun): boolean { } function hasRuntimeOnlyFollowupMetadata(item: FollowupRun): boolean { - return Boolean( - hasCurrentTurnRuntimeMetadata(item) || - item.abortSignal || - item.deliveryCorrelations?.length || - item.queuedLifecycle, + return item.currentInboundEventKind === "room_event" || item.currentInboundAudio === true; +} + +function buildCollectTranscriptPrompt(items: FollowupRun[]): string { + return buildCollectPrompt({ + title: "[Queued messages while agent was busy]", + items, + renderItem: (item, index) => + renderCollectItemPrompt(item, index, item.transcriptPrompt ?? item.prompt), + }); +} + +function resolveFollowupTranscriptTarget(source: FollowupRun) { + const sessionKey = normalizeOptionalString(source.run.sessionKey); + const storePath = sessionKey + ? resolveStorePath(source.run.config.session?.store, { + agentId: source.run.agentId, + }) + : undefined; + if (!sessionKey || !storePath) { + return { + transcriptPath: source.run.sessionFile, + sessionId: source.run.sessionId, + agentId: source.run.agentId, + sessionKey: source.run.sessionId, + cwd: source.run.cwd ?? source.run.workspaceDir, + config: source.run.config, + }; + } + const sessionEntry = loadSessionEntry({ + storePath, + sessionKey, + clone: false, + }); + return { + sessionId: sessionEntry?.sessionId ?? source.run.sessionId, + sessionKey, + sessionEntry, + storePath, + agentId: source.run.agentId, + cwd: source.run.cwd ?? source.run.workspaceDir, + config: source.run.config, + }; +} + +function createCollectUserTurnTranscriptRecorder(items: FollowupRun[]) { + const transcriptSources = items.filter((item) => item.userTurnTranscriptRecorder); + const source = transcriptSources.at(-1); + if (!source) { + return undefined; + } + const buildInput = async () => { + const messages = await Promise.all( + transcriptSources.map( + async (item) => await item.userTurnTranscriptRecorder?.resolveMessage(), + ), + ); + const media = messages.flatMap((message) => + buildPersistedUserTurnMediaInputsFromFields(message), + ); + const timestamp = messages.reduce((latest, message) => { + const candidate = message?.timestamp; + return typeof candidate === "number" && (latest === undefined || candidate > latest) + ? candidate + : latest; + }, undefined); + const transcriptPrompt = buildCollectTranscriptPrompt(transcriptSources); + const identityHash = createHash("sha256") + .update( + JSON.stringify( + transcriptSources.map((item) => [ + item.messageId ?? "", + item.enqueuedAt, + item.transcriptPrompt, + ]), + ), + ) + .digest("hex"); + return { + text: transcriptPrompt, + senderIsOwner: source.run.senderIsOwner, + provenance: source.run.inputProvenance, + idempotencyKey: `followup-collect:${source.run.sessionId}:${identityHash}`, + ...(timestamp === undefined ? {} : { timestamp }), + ...(media.length === 0 + ? {} + : { + media, + mediaOnlyText: "[User sent media without caption]", + }), + }; + }; + const initialTranscriptPrompt = buildCollectTranscriptPrompt(transcriptSources); + return createUserTurnTranscriptRecorder({ + input: { + text: initialTranscriptPrompt, + senderIsOwner: source.run.senderIsOwner, + provenance: source.run.inputProvenance, + }, + resolveInput: buildInput, + target: () => resolveFollowupTranscriptTarget(source), + errorContext: "collected followup user turn transcript", + beforeMessageWrite: runAgentHarnessBeforeMessageWriteHook, + }); +} + +function resolveAggregateOwner(items: readonly FollowupRun[]): FollowupRun | undefined { + // Keep the latest cancelable source as the aggregate owner even when a + // later transport-only source has no cancellation identity. + return ( + items.findLast((item) => item.abortSignal) ?? + items.findLast((item) => item.queuedLifecycle) ?? + items.at(-1) ); } -function combineAbortSignals(items: readonly FollowupRun[]): AbortSignal | undefined { - const signals = items.flatMap((item) => - [item.abortSignal, item.queueAbortSignal].filter( - (signal): signal is AbortSignal => signal !== undefined, - ), - ); - if (signals.length === 0) { - return undefined; - } - if (signals.length === 1) { - return signals[0]; - } - const nativeAny = ( - AbortSignal as typeof AbortSignal & { - any?: (signals: AbortSignal[]) => AbortSignal; +type AggregateCancellation = { + signal?: AbortSignal; + admit: () => void; + dispose: () => void; +}; + +function createAggregateCancellation(items: readonly FollowupRun[]): AggregateCancellation { + const owner = resolveAggregateOwner(items); + const sourceSignals = new Map>(); + for (const item of items) { + if (!item.abortSignal) { + continue; } - ).any; - if (nativeAny) { - return nativeAny(signals); + const owners = sourceSignals.get(item.abortSignal) ?? new Set(); + owners.add(item); + sourceSignals.set(item.abortSignal, owners); + } + const signals = new Set(sourceSignals.keys()); + if (signals.size === 0) { + return { + signal: undefined, + admit: () => undefined, + dispose: () => undefined, + }; + } + const onlySignal = signals.size === 1 ? signals.values().next().value : undefined; + const onlySignalOwned = + onlySignal && owner ? sourceSignals.get(onlySignal)?.has(owner) === true : false; + if (onlySignal && onlySignalOwned) { + return { + signal: onlySignal, + admit: () => undefined, + dispose: () => undefined, + }; } const controller = new AbortController(); - const abort = () => controller.abort(); + const listeners = new Map void>(); for (const signal of signals) { + const abort = () => controller.abort(); + listeners.set(signal, abort); if (signal.aborted) { abort(); - break; + } else { + signal.addEventListener("abort", abort, { once: true }); } - signal.addEventListener("abort", abort, { once: true }); } - return controller.signal; + const disposeSignal = (signal: AbortSignal) => { + const listener = listeners.get(signal); + if (!listener) { + return; + } + signal.removeEventListener("abort", listener); + listeners.delete(signal); + }; + return { + signal: controller.signal, + admit: () => { + // Before admission every source remains independently cancellable. Once + // atomic, only the latest source owns aggregate client cancellation. + for (const [signal, sourceOwners] of sourceSignals) { + if (!owner || !sourceOwners.has(owner)) { + disposeSignal(signal); + } + } + }, + dispose: () => { + for (const signal of listeners.keys()) { + disposeSignal(signal); + } + }, + }; +} + +function collectCurrentInboundContext(items: FollowupRun[]): FollowupRun["currentInboundContext"] { + const contexts = items.flatMap((item, index) => + item.currentInboundContext ? [{ context: item.currentInboundContext, index }] : [], + ); + if (contexts.length === 0) { + return undefined; + } + if (contexts.length === 1) { + return contexts[0]?.context; + } + const renderField = (field: "text" | "resumableText") => { + const blocks = contexts.flatMap(({ context, index }) => { + const value = context[field]; + return value ? [`Queued #${index + 1} context:\n${value}`] : []; + }); + return blocks.length > 0 ? blocks.join("\n\n") : undefined; + }; + const text = renderField("text"); + if (!text) { + return undefined; + } + const resumableText = renderField("resumableText"); + return { + text, + ...(resumableText ? { resumableText } : {}), + promptJoiner: "\n\n", + }; } function collectRuntimeMetadata( items: FollowupRun[], - singletonOwner?: FollowupRun, + abortSignal?: AbortSignal, ): FollowupRuntimeMetadata { - const candidates = singletonOwner ? [singletonOwner, ...items] : items; - const currentTurnSource = - singletonOwner && hasCurrentTurnRuntimeMetadata(singletonOwner) - ? singletonOwner - : items.find(hasCurrentTurnRuntimeMetadata); - const abortSignal = singletonOwner?.abortSignal ?? combineAbortSignals(candidates); - const queueAbortSignal = singletonOwner?.queueAbortSignal; + const currentTurnSource = items.find(hasCurrentTurnRuntimeMetadata); const deliveryCorrelations = items.flatMap((item) => item.deliveryCorrelations ?? []); - const lifecycleSource = singletonOwner ?? items.find((item) => item.queuedLifecycle); return { currentInboundEventKind: currentTurnSource?.currentInboundEventKind, currentInboundAudio: currentTurnSource?.currentInboundAudio, - currentInboundContext: currentTurnSource?.currentInboundContext, + currentInboundContext: collectCurrentInboundContext(items), abortSignal, - queueAbortSignal, + queueAbortSignal: items.find((item) => item.queueAbortSignal)?.queueAbortSignal, deliveryCorrelations: deliveryCorrelations.length > 0 ? deliveryCorrelations : undefined, - queuedLifecycle: - singletonOwner?.queuedLifecycle ?? - (items.length === 1 ? lifecycleSource?.queuedLifecycle : undefined), + queuedLifecycle: items.length === 1 ? items[0]?.queuedLifecycle : undefined, }; } type FollowupQueueSummaryState = { + cap: number; dropPolicy: "summarize" | "old" | "new"; droppedCount: number; summaryLines: string[]; summarySources: FollowupRun[]; + activeSummarySources: WeakSet; summaryElisions: Array<{ contextKey: string; count: number; - source: FollowupRun; - sourceRefs: WeakSet; - allRoomEvents: boolean; + sources: FollowupRun[]; + sourceRefs: WeakMap; }>; evictedSummaryCount: number; }; -function clearFollowupQueueSummaryState(queue: FollowupQueueSummaryState): void { - completeFollowupQueueSummarySources(queue); - for (const entry of queue.summaryElisions) { - completeFollowupRunLifecycle(entry.source); - } - queue.summaryElisions = []; - queue.evictedSummaryCount = 0; - clearQueueSummaryState(queue); -} - -function completeFollowupQueueSummarySources(queue: { summarySources?: FollowupRun[] }): void { - for (const item of queue.summarySources ?? []) { - completeFollowupRunLifecycle(item); - } - if (queue.summarySources) { - queue.summarySources = []; - } -} - type QueueSummaryDelivery = { prompt: string; droppedCount: number; @@ -387,6 +547,7 @@ function createQueueSummaryDelivery(params: { function consumeQueueSummaryDelivery( queue: FollowupQueueSummaryState, delivery: QueueSummaryDelivery, + completeLifecycles = true, ): void { let consumedCount = delivery.sources.length === 0 ? delivery.droppedCount : 0; for (const source of delivery.sources) { @@ -396,17 +557,23 @@ function consumeQueueSummaryDelivery( queue.summaryLines.splice(sourceIndex, 1); consumedCount += 1; } else { - const elisionIndex = queue.summaryElisions.findIndex((entry) => entry.sourceRefs.has(source)); + const elisionIndex = queue.summaryElisions.findIndex( + (entry) => entry.sources.includes(source) || entry.sourceRefs.has(source), + ); if (elisionIndex >= 0) { const entry = queue.summaryElisions[elisionIndex]; - entry.count = Math.max(0, entry.count - 1); + const elidedSourceIndex = entry.sources.indexOf(entry.sourceRefs.get(source) ?? source); + entry.sources.splice(elidedSourceIndex, 1); + entry.count = entry.sources.length; consumedCount += 1; - if (entry.count === 0) { + if (entry.sources.length === 0) { queue.summaryElisions.splice(elisionIndex, 1); } } } - completeFollowupRunLifecycle(source); + if (completeLifecycles) { + completeFollowupRunLifecycle(source); + } } queue.droppedCount = Math.max(0, queue.droppedCount - consumedCount); } @@ -420,24 +587,116 @@ function releaseQueueSummaryDeliveryForRetry( if (sourceIndex >= 0) { queue.summarySources[sourceIndex] = createOverflowSummaryRetrySource(source); } - completeFollowupRunLifecycle(source); + if (!source.queuedLifecycle) { + completeFollowupRunLifecycle(source); + } } } +function dropAbortedQueueSummarySources(queue: FollowupQueueSummaryState): number { + let dropped = 0; + for (let index = queue.summarySources.length - 1; index >= 0; index -= 1) { + const source = queue.summarySources[index]; + if (!isFollowupRunAborted(source)) { + continue; + } + queue.summarySources.splice(index, 1); + queue.summaryLines.splice(index, 1); + queue.droppedCount = Math.max(0, queue.droppedCount - 1); + completeFollowupRunLifecycle(source); + dropped += 1; + } + return dropped; +} + async function runQueueSummaryDelivery( queue: FollowupQueueSummaryState, delivery: QueueSummaryDelivery, - run: () => Promise, -): Promise { - try { - await run(); - } catch (err) { - if (!isFollowupRunDeferredError(err)) { - releaseQueueSummaryDeliveryForRetry(queue, delivery); - } - throw err; + run: (params: { abortSignal?: AbortSignal; onAdmitted?: () => void }) => Promise, + protectedSources: FollowupRun[] = delivery.sources, +): Promise { + const inheritedActiveSources = new Set( + protectedSources.filter((source) => queue.activeSummarySources.has(source)), + ); + for (const source of protectedSources) { + queue.activeSummarySources.add(source); + } + let admitted = false; + let deferredBeforeAdmission = false; + const cancellation = createAggregateCancellation(protectedSources); + const onAdmitted = + protectedSources.length > 1 + ? () => { + if (admitted) { + return; + } + cancellation.admit(); + admitted = true; + // A multi-source summary is atomic once it owns the reply lane. + // Retire sibling ids while the latest source owns aggregate cancel. + consumeQueueSummaryDelivery(queue, { ...delivery, sources: protectedSources }, false); + const aggregateOwner = resolveAggregateOwner(protectedSources); + for (const source of protectedSources) { + if (source !== aggregateOwner) { + retireFollowupRunCancellation(source); + } + } + } + : undefined; + try { + try { + await run({ abortSignal: cancellation.signal, onAdmitted }); + } catch (err) { + if (!admitted) { + deferredBeforeAdmission = isFollowupRunDeferredError(err); + if (!deferredBeforeAdmission) { + releaseQueueSummaryDeliveryForRetry(queue, delivery); + } + } else { + // Admission consumed the aggregate sources, so a failed attempt is + // terminal for their queue identities rather than retryable queue work. + for (const source of protectedSources) { + completeFollowupRunLifecycle(source); + } + } + throw err; + } + if (!admitted) { + const canceledSources = protectedSources.filter(isFollowupRunAborted); + if (canceledSources.length > 0) { + consumeQueueSummaryDelivery(queue, { + ...delivery, + sources: canceledSources, + }); + return false; + } + } + if (!admitted) { + consumeQueueSummaryDelivery(queue, delivery); + } + return true; + } finally { + cancellation.dispose(); + // Carry one deferred generation across retries. Later retries release newly + // protected sources so continued overflow cannot grow retained identities. + const deferredCarryover = + deferredBeforeAdmission && inheritedActiveSources.size === 0 + ? new Set(protectedSources) + : inheritedActiveSources; + for (const source of protectedSources) { + if (deferredBeforeAdmission && deferredCarryover.has(source)) { + continue; + } + queue.activeSummarySources.delete(source); + for (const entry of queue.summaryElisions) { + const compactSource = entry.sourceRefs.get(source); + if (compactSource) { + queue.activeSummarySources.delete(compactSource); + } + } + } + trimSummaryElisionsToCap(queue); } - consumeQueueSummaryDelivery(queue, delivery); } async function dropAbortedFollowups( @@ -473,7 +732,21 @@ function resolveCrossChannelKey(item: FollowupRun): { cross?: true; key?: string return chatType ? { key: JSON.stringify(["unresolved", chatType]) } : {}; } if (!isRoutableChannel(channel) || !to) { - return { cross: true }; + // Internal/local transports (notably webchat) have no external destination. + // Keep their full route identity so matching turns can collect safely. + return { + key: JSON.stringify([ + "local", + channel ?? "", + to ?? "", + accountId ?? "", + threadId ?? "", + item.originatingChatId ?? "", + replyToId ?? "", + item.originatingReplyToMode ?? "", + chatType ?? "", + ]), + }; } const key = channelRouteCompactKey({ channel, to, accountId, threadId }); return key @@ -522,6 +795,8 @@ export function createOverflowSummaryRetrySource(source: FollowupRun): FollowupR originatingReplyToId: source.originatingReplyToId, originatingReplyToMode: source.originatingReplyToMode, originatingChatType: source.originatingChatType, + abortSignal: source.abortSignal, + queuedLifecycle: source.queuedLifecycle, ...(source.currentInboundEventKind === "room_event" ? { currentInboundEventKind: "room_event" } : {}), @@ -540,6 +815,8 @@ async function runSyntheticOverflowSummary(params: { source: FollowupRun; sources: FollowupRun[]; prompt: string; + abortSignal?: AbortSignal; + onAdmitted?: () => void; runFollowup: (run: FollowupRun) => Promise; }): Promise { const promptHash = createHash("sha256").update(params.prompt).digest("hex"); @@ -558,48 +835,18 @@ async function runSyntheticOverflowSummary(params: { ]), ) .digest("hex"); - const sessionKey = normalizeOptionalString(params.source.run.sessionKey); - const storePath = sessionKey - ? resolveStorePath(params.source.run.config.session?.store, { - agentId: params.source.run.agentId, - }) - : undefined; const userTurnTranscriptRecorder = createUserTurnTranscriptRecorder({ input: { text: params.prompt, idempotencyKey: `followup-overflow:${params.source.run.sessionId}:${routeHash}:${params.source.messageId ?? params.source.enqueuedAt}:${promptHash}`, provenance: params.source.run.inputProvenance, }, - target: () => { - if (!sessionKey || !storePath) { - return { - transcriptPath: params.source.run.sessionFile, - sessionId: params.source.run.sessionId, - agentId: params.source.run.agentId, - sessionKey: params.source.run.sessionId, - cwd: params.source.run.cwd ?? params.source.run.workspaceDir, - config: params.source.run.config, - }; - } - const sessionEntry = loadSessionEntry({ - storePath, - sessionKey, - clone: false, - }); - return { - sessionId: sessionEntry?.sessionId ?? params.source.run.sessionId, - sessionKey, - sessionEntry, - storePath, - agentId: params.source.run.agentId, - cwd: params.source.run.cwd ?? params.source.run.workspaceDir, - config: params.source.run.config, - }; - }, + target: () => resolveFollowupTranscriptTarget(params.source), beforeMessageWrite: runAgentHarnessBeforeMessageWriteHook, errorContext: "followup overflow summary transcript", }); const currentInboundEventKind = resolveOverflowSummaryInboundEventKind(params.sources); + let admitted = false; await params.runFollowup({ prompt: params.prompt, queueAbortSignal: params.source.queueAbortSignal, @@ -608,6 +855,24 @@ async function runSyntheticOverflowSummary(params: { userTurnTranscriptRecorder, run: params.source.run, enqueuedAt: Date.now(), + abortSignal: params.abortSignal, + ...(params.onAdmitted + ? { + queuedLifecycle: { + onAdmitted: () => { + admitted = true; + params.onAdmitted?.(); + }, + onComplete: () => { + if (admitted) { + for (const source of params.sources) { + completeFollowupRunLifecycle(source); + } + } + }, + }, + } + : {}), ...resolveOriginRoutingMetadata([params.source]), ...(currentInboundEventKind ? { currentInboundEventKind } : {}), }); @@ -627,8 +892,26 @@ async function drainElidedOverflowSummary(params: { (source) => resolveFollowupDeliveryContextKey(source) === entry.contextKey, ) : []; - const source = retainedSources.at(-1) ?? entry.source; - const elidedCount = entry.count; + for (let index = entry.sources.length - 1; index >= 0; index -= 1) { + const source = entry.sources[index]; + if (!isFollowupRunAborted(source)) { + continue; + } + entry.sources.splice(index, 1); + entry.count = Math.max(0, entry.count - 1); + params.queue.droppedCount = Math.max(0, params.queue.droppedCount - 1); + completeFollowupRunLifecycle(source); + } + if (entry.sources.length === 0) { + params.queue.summaryElisions.shift(); + return true; + } + const source = retainedSources.at(-1) ?? entry.sources.at(-1); + if (!source) { + return false; + } + const elidedCount = entry.sources.length; + const elidedSources = [...entry.sources]; const droppedCount = elidedCount + retainedSources.length; const summaryLines = params.queue.summaryLines.slice(0, retainedSources.length); const prompt = previewQueueSummaryPrompt({ @@ -642,30 +925,40 @@ async function drainElidedOverflowSummary(params: { if (!prompt) { return false; } - await runQueueSummaryDelivery( + const delivered = await runQueueSummaryDelivery( params.queue, { prompt, droppedCount: retainedSources.length, sources: retainedSources, }, - async () => { + async ({ abortSignal, onAdmitted }) => { await runSyntheticOverflowSummary({ source, - sources: entry.allRoomEvents ? [entry.source, ...retainedSources] : [], + sources: [...elidedSources, ...retainedSources], prompt, + abortSignal, + onAdmitted, runFollowup: params.runFollowup, }); }, + [...elidedSources, ...retainedSources], ); + if (!delivered) { + return true; + } const entryIndex = params.queue.summaryElisions.indexOf(entry); if (entryIndex < 0) { return true; } - const consumedCount = Math.min(elidedCount, entry.count); - entry.count -= consumedCount; + const consumedCount = Math.min(elidedCount, entry.sources.length); + const consumedSources = entry.sources.splice(0, consumedCount); + entry.count = entry.sources.length; + for (const consumedSource of consumedSources) { + completeFollowupRunLifecycle(consumedSource); + } params.queue.droppedCount = Math.max(0, params.queue.droppedCount - consumedCount); - if (entry.count === 0) { + if (entry.sources.length === 0) { params.queue.summaryElisions.splice(entryIndex, 1); } return true; @@ -675,6 +968,9 @@ async function drainOverflowSummaryGroup(params: { queue: FollowupQueueSummaryState; runFollowup: (run: FollowupRun) => Promise; }): Promise { + if (dropAbortedQueueSummarySources(params.queue) > 0 && params.queue.droppedCount === 0) { + return true; + } if (params.queue.evictedSummaryCount > 0) { const evictedCount = params.queue.evictedSummaryCount; params.queue.evictedSummaryCount = 0; @@ -699,11 +995,13 @@ async function drainOverflowSummaryGroup(params: { if (!delivery) { return false; } - await runQueueSummaryDelivery(params.queue, delivery, async () => { + await runQueueSummaryDelivery(params.queue, delivery, async ({ abortSignal, onAdmitted }) => { await runSyntheticOverflowSummary({ source, sources: delivery.sources, prompt: delivery.prompt, + abortSignal, + onAdmitted, runFollowup: params.runFollowup, }); }); @@ -734,18 +1032,12 @@ export function scheduleFollowupDrain( try { const collectState = { forceIndividualCollect: false }; while (queue.items.length > 0 || queue.droppedCount > 0) { - const droppedBeforeDebounce = await dropAbortedFollowups(queue.items, effectiveRunFollowup); - if (droppedBeforeDebounce > 0 && queue.items.length === 0) { - clearFollowupQueueSummaryState(queue); - } + await dropAbortedFollowups(queue.items, effectiveRunFollowup); if (queue.items.length === 0 && queue.droppedCount === 0) { break; } await waitForQueueDebounce(queue); - const droppedAfterDebounce = await dropAbortedFollowups(queue.items, effectiveRunFollowup); - if (droppedAfterDebounce > 0 && queue.items.length === 0) { - clearFollowupQueueSummaryState(queue); - } + await dropAbortedFollowups(queue.items, effectiveRunFollowup); if (queue.items.length === 0 && queue.droppedCount === 0) { break; } @@ -792,33 +1084,99 @@ export function scheduleFollowupDrain( } for (const groupItems of contextGroups) { - const groupSource = groupItems.at(-1); + const abortedGroupItems = groupItems.filter(isFollowupRunAborted); + if (abortedGroupItems.length > 0) { + removeQueuedItemsByRef(queue.items, abortedGroupItems); + for (const item of abortedGroupItems) { + completeFollowupRunLifecycle(item); + } + } + const activeGroupItems = groupItems.filter((item) => !isFollowupRunAborted(item)); + if (activeGroupItems.length === 0) { + continue; + } + const groupSource = activeGroupItems.at(-1); const run = groupSource?.run ?? queue.lastRun; if (!run) { break; } - const routing = resolveOriginRoutingMetadata(groupItems); + const routing = resolveOriginRoutingMetadata(activeGroupItems); const prompt = buildCollectPrompt({ title: "[Queued messages while agent was busy]", - items: groupItems, + items: activeGroupItems, renderItem: renderCollectItem, }); + const transcriptPrompt = buildCollectTranscriptPrompt(activeGroupItems); + const userTurnTranscriptRecorder = + createCollectUserTurnTranscriptRecorder(activeGroupItems); + const aggregateOwner = resolveAggregateOwner(activeGroupItems); + const cancellation = createAggregateCancellation(activeGroupItems); + let admitted = false; + const consumeAdmittedGroup = () => { + cancellation.admit(); + admitted = true; + removeQueuedItemsByRef(queue.items, activeGroupItems); + for (const item of activeGroupItems) { + if (item !== aggregateOwner) { + retireFollowupRunCancellation(item); + } + } + }; + const completeGroup = () => { + removeQueuedItemsByRef(queue.items, activeGroupItems); + for (const item of activeGroupItems) { + completeFollowupRunLifecycle(item); + } + }; const drainGroup = async () => { await effectiveRunFollowup({ prompt, + transcriptPrompt, + ...(userTurnTranscriptRecorder ? { userTurnTranscriptRecorder } : {}), run, messageId: groupSource?.messageId ?? (groupSource ? resolveFollowupReplyAnchor(groupSource) : undefined), enqueuedAt: Date.now(), ...routing, - ...collectRuntimeMetadata(groupItems), - ...collectQueuedImages(groupItems), + ...collectRuntimeMetadata(activeGroupItems, cancellation.signal), + ...(activeGroupItems.length > 1 + ? { + queuedLifecycle: { + onAdmitted: consumeAdmittedGroup, + onComplete: () => { + if (admitted) { + completeGroup(); + } + }, + }, + } + : {}), + ...collectQueuedImages(activeGroupItems), }); }; - await drainGroup(); - removeQueuedItemsByRef(queue.items, groupItems); + try { + await drainGroup(); + } catch (err) { + if (admitted) { + completeGroup(); + } + throw err; + } finally { + cancellation.dispose(); + } + if (!admitted) { + const canceledSources = activeGroupItems.filter(isFollowupRunAborted); + if (canceledSources.length > 0) { + removeQueuedItemsByRef(queue.items, canceledSources); + for (const item of canceledSources) { + completeFollowupRunLifecycle(item); + } + continue; + } + } + completeGroup(); } continue; } diff --git a/src/auto-reply/reply/queue/enqueue.ts b/src/auto-reply/reply/queue/enqueue.ts index 33588d6fe75..24a0406968c 100644 --- a/src/auto-reply/reply/queue/enqueue.ts +++ b/src/auto-reply/reply/queue/enqueue.ts @@ -11,7 +11,7 @@ import { resolveFollowupDeliveryContextKey, resolveFollowupReplyAnchor, } from "./drain.js"; -import { getExistingFollowupQueue, getFollowupQueue } from "./state.js"; +import { getExistingFollowupQueue, getFollowupQueue, trimSummaryElisionsToCap } from "./state.js"; import { completeFollowupRunLifecycle, isFollowupRunAborted, @@ -118,6 +118,15 @@ export function enqueueFollowupRun( if (shouldSkipQueueItem({ item: run, items: queue.items, dedupe })) { return false; } + // drop:new rejects this source without mutating the existing queue. Do not + // publish an external queued identity for work that will never be admitted. + if (queue.dropPolicy === "new" && queue.cap > 0 && queue.items.length >= queue.cap) { + completeFollowupRunLifecycle(run); + return false; + } + if (!markFollowupRunEnqueued(run)) { + return false; + } queue.lastEnqueuedAt = Date.now(); queue.lastRun = run.run; @@ -142,38 +151,36 @@ export function enqueueFollowupRun( const contextKey = resolveFollowupDeliveryContextKey(item); const lastElision = queue.summaryElisions.at(-1); if (lastElision?.contextKey === contextKey) { + const compactSource = createOverflowSummaryRetrySource(item); lastElision.count += 1; - lastElision.source = createOverflowSummaryRetrySource(item); - lastElision.sourceRefs.add(item); - lastElision.allRoomEvents = - lastElision.allRoomEvents && item.currentInboundEventKind === "room_event"; - } else { - if (queue.summaryElisions.length >= queue.cap) { - const evicted = queue.summaryElisions.shift(); - if (evicted) { - queue.evictedSummaryCount += evicted.count; - completeFollowupRunLifecycle(evicted.source); - } + lastElision.sources.push(compactSource); + lastElision.sourceRefs.set(item, compactSource); + if (queue.activeSummarySources.has(item)) { + queue.activeSummarySources.add(compactSource); } + } else { + const compactSource = createOverflowSummaryRetrySource(item); queue.summaryElisions.push({ contextKey, count: 1, - source: createOverflowSummaryRetrySource(item), - sourceRefs: new WeakSet([item]), - allRoomEvents: item.currentInboundEventKind === "room_event", + sources: [compactSource], + sourceRefs: new WeakMap([[item, compactSource]]), }); + if (queue.activeSummarySources.has(item)) { + queue.activeSummarySources.add(compactSource); + } } - completeFollowupRunLifecycle(item); + trimSummaryElisionsToCap(queue); } } } if (!shouldEnqueue) { + completeFollowupRunLifecycle(run); return false; } run.queueAbortSignal = queue.abortController.signal; queue.items.push(run); - markFollowupRunEnqueued(run); if (recentMessageIdKey) { RECENT_QUEUE_MESSAGE_IDS.check(recentMessageIdKey); } diff --git a/src/auto-reply/reply/queue/state.test.ts b/src/auto-reply/reply/queue/state.test.ts index d255c31c82a..e9ecf896599 100644 --- a/src/auto-reply/reply/queue/state.test.ts +++ b/src/auto-reply/reply/queue/state.test.ts @@ -48,13 +48,14 @@ describe("refreshQueuedFollowupSession", () => { queue.summaryElisions.push({ contextKey: "context", count: 2, - source: { - prompt: "elided summary", - enqueuedAt: Date.now(), - run: makeRun(), - }, - sourceRefs: new WeakSet(), - allRoomEvents: false, + sources: [ + { + prompt: "elided summary", + enqueuedAt: Date.now(), + run: makeRun(), + }, + ], + sourceRefs: new WeakMap(), }); refreshQueuedFollowupSession({ @@ -86,7 +87,7 @@ describe("refreshQueuedFollowupSession", () => { authProfileId: undefined, authProfileIdSource: undefined, }); - expect(queue.summaryElisions[0]?.source.run).toEqual({ + expect(queue.summaryElisions[0]?.sources[0]?.run).toEqual({ ...makeRun(), provider: "openai", model: "gpt-4o", @@ -145,13 +146,12 @@ describe("getFollowupQueue", () => { queue.summaryElisions.push({ contextKey, count, - source: { + sources: Array.from({ length: count }, () => ({ prompt: contextKey, enqueuedAt: Date.now(), run: makeRun(), - }, - sourceRefs: new WeakSet(), - allRoomEvents: false, + })), + sourceRefs: new WeakMap(), }); } queue.evictedSummaryCount = 5; @@ -159,6 +159,7 @@ describe("getFollowupQueue", () => { const updated = getFollowupQueue(QUEUE_KEY, { mode: "followup", cap: 1 }); expect(updated.summaryElisions.map((entry) => entry.contextKey)).toEqual(["newest"]); - expect(updated.evictedSummaryCount).toBe(10); + expect(updated.summaryElisions[0]?.sources).toHaveLength(1); + expect(updated.evictedSummaryCount).toBe(13); }); }); diff --git a/src/auto-reply/reply/queue/state.ts b/src/auto-reply/reply/queue/state.ts index c747035200d..a5b04013502 100644 --- a/src/auto-reply/reply/queue/state.ts +++ b/src/auto-reply/reply/queue/state.ts @@ -22,12 +22,15 @@ export type FollowupQueueState = { droppedCount: number; summaryLines: string[]; summarySources: FollowupRun[]; + /** Sources currently used by an async summary delivery cannot be evicted mid-run. */ + activeSummarySources: WeakSet; summaryElisions: Array<{ contextKey: string; count: number; - source: FollowupRun; - sourceRefs: WeakSet; - allRoomEvents: boolean; + /** Compact sources stay strong so cancellation follows summarized content until delivery. */ + sources: FollowupRun[]; + /** Weak source mapping keeps concurrent summary consumption identity-safe. */ + sourceRefs: WeakMap; }>; evictedSummaryCount: number; lastRun?: FollowupRun["run"]; @@ -53,14 +56,44 @@ export function getExistingFollowupQueue(key: string): FollowupQueueState | unde return FOLLOWUP_QUEUES.get(cleaned); } -function trimSummaryElisionsToCap(queue: FollowupQueueState): void { - while (queue.summaryElisions.length > queue.cap) { - const evicted = queue.summaryElisions.shift(); +type SummaryElisionCapState = Pick< + FollowupQueueState, + "activeSummarySources" | "cap" | "evictedSummaryCount" | "summaryElisions" +>; + +export function trimSummaryElisionsToCap(queue: SummaryElisionCapState): void { + let sourceCount = queue.summaryElisions.reduce( + (count, entry) => + count + entry.sources.filter((source) => !queue.activeSummarySources.has(source)).length, + 0, + ); + while (sourceCount > queue.cap) { + let evicted = false; + for (let entryIndex = 0; entryIndex < queue.summaryElisions.length; entryIndex += 1) { + const entry = queue.summaryElisions[entryIndex]; + const sourceIndex = entry.sources.findIndex( + (source) => !queue.activeSummarySources.has(source), + ); + if (sourceIndex < 0) { + continue; + } + const [source] = entry.sources.splice(sourceIndex, 1); + entry.count = entry.sources.length; + queue.evictedSummaryCount += 1; + sourceCount -= 1; + if (source) { + completeFollowupRunLifecycle(source); + } + if (entry.sources.length === 0) { + queue.summaryElisions.splice(entryIndex, 1); + } + evicted = true; + break; + } if (!evicted) { + // A deferred delivery temporarily retains at most one queue-cap-sized active set. return; } - queue.evictedSummaryCount += evicted.count; - completeFollowupRunLifecycle(evicted.source); } } @@ -93,6 +126,7 @@ export function getFollowupQueue(key: string, settings: QueueSettings): Followup droppedCount: 0, summaryLines: [], summarySources: [], + activeSummarySources: new WeakSet(), summaryElisions: [], evictedSummaryCount: 0, }; @@ -119,7 +153,9 @@ export function clearFollowupQueue(key: string): number { completeFollowupRunLifecycle(item); } for (const entry of queue.summaryElisions) { - completeFollowupRunLifecycle(entry.source); + for (const source of entry.sources) { + completeFollowupRunLifecycle(source); + } } queue.items.length = 0; queue.droppedCount = 0; @@ -210,6 +246,8 @@ export function refreshQueuedFollowupSession(params: { rewriteRun(item.run); } for (const entry of queue.summaryElisions) { - rewriteRun(entry.source.run); + for (const source of entry.sources) { + rewriteRun(source.run); + } } } diff --git a/src/auto-reply/reply/queue/types.ts b/src/auto-reply/reply/queue/types.ts index 8b44c247bdb..d7cc3ca8675 100644 --- a/src/auto-reply/reply/queue/types.ts +++ b/src/auto-reply/reply/queue/types.ts @@ -169,15 +169,28 @@ export function isFollowupRunAborted( } const enqueuedFollowupLifecycles = new WeakSet(); +const retiredFollowupCancellationLifecycles = new WeakSet(); const completedFollowupLifecycles = new WeakSet(); -export function markFollowupRunEnqueued(run: Pick): void { +export function markFollowupRunEnqueued(run: Pick): boolean { const lifecycle = run.queuedLifecycle; if (!lifecycle || enqueuedFollowupLifecycles.has(lifecycle)) { - return; + return true; + } + if (lifecycle.onEnqueued?.() === false) { + return false; } enqueuedFollowupLifecycles.add(lifecycle); - lifecycle.onEnqueued?.(); + return true; +} + +export function retireFollowupRunCancellation(run: Pick): void { + const lifecycle = run.queuedLifecycle; + if (!lifecycle || retiredFollowupCancellationLifecycles.has(lifecycle)) { + return; + } + retiredFollowupCancellationLifecycles.add(lifecycle); + lifecycle.onCancellationRetired?.(); } export function completeFollowupRunLifecycle(run: Pick): void { diff --git a/src/gateway/chat-abort.ts b/src/gateway/chat-abort.ts index e7e8c07da3c..393855e9bab 100644 --- a/src/gateway/chat-abort.ts +++ b/src/gateway/chat-abort.ts @@ -61,6 +61,8 @@ export type ChatAbortControllerEntry = { * is active" must check `kind !== "agent"`, not just `.has(runId)`. */ kind?: "chat-send" | "agent"; + /** Side questions stay independent from main-turn TUI session stops. */ + turnKind?: "main" | "btw"; }; export type RestartRecoveryCandidate = { @@ -153,6 +155,7 @@ export function registerChatAbortController(params: { isAbortable?: (entry: ChatAbortControllerEntry) => boolean; onRemoved?: () => void; kind?: ChatAbortControllerEntry["kind"]; + turnKind?: ChatAbortControllerEntry["turnKind"]; lifecycleGeneration?: string; now?: number; expiresAtMs?: number; @@ -219,6 +222,7 @@ export function registerChatAbortController(params: { onRemoved: params.onRemoved, projectSessionActive: true, kind: params.kind, + turnKind: params.turnKind, }; params.chatAbortControllers.set(params.runId, entry); return { controller, registered: true, entry, cleanup }; diff --git a/src/gateway/chat-queued-turns.test.ts b/src/gateway/chat-queued-turns.test.ts new file mode 100644 index 00000000000..2a55cb9ae6c --- /dev/null +++ b/src/gateway/chat-queued-turns.test.ts @@ -0,0 +1,222 @@ +import { describe, expect, it } from "vitest"; +import { + abortQueuedChatTurnById, + abortQueuedChatTurns, + completeQueuedChatTurn, + getQueuedChatTurn, + listQueuedChatTurnsForSession, + registerQueuedChatTurn, + retireQueuedChatTurnCancellation, + type QueuedChatTurnMap, +} from "./chat-queued-turns.js"; + +function emptyMap(): QueuedChatTurnMap { + return new Map(); +} + +describe("chat-queued-turns", () => { + it("registers and completes a queued turn", () => { + const map = emptyMap(); + const controller = new AbortController(); + expect( + registerQueuedChatTurn({ + chatQueuedTurns: map, + runId: "run-a", + controller, + sessionId: "sess-a", + sessionKey: "main", + ownerConnId: "conn-1", + ownerDeviceId: "dev-1", + }), + ).toBe(true); + expect(getQueuedChatTurn(map, "run-a")?.sessionKey).toBe("main"); + expect(completeQueuedChatTurn(map, "run-a")).toBe(true); + expect(getQueuedChatTurn(map, "run-a")).toBeUndefined(); + }); + + it("rejects re-register with a different controller", () => { + const map = emptyMap(); + const first = new AbortController(); + registerQueuedChatTurn({ + chatQueuedTurns: map, + runId: "run-a", + controller: first, + sessionId: "sess-a", + sessionKey: "main", + }); + expect( + registerQueuedChatTurn({ + chatQueuedTurns: map, + runId: "run-a", + controller: new AbortController(), + sessionId: "sess-a", + sessionKey: "main", + }), + ).toBe(false); + }); + + it("preserves whitespace-distinct protocol run IDs", () => { + const map = emptyMap(); + const spaced = new AbortController(); + const plain = new AbortController(); + expect( + registerQueuedChatTurn({ + chatQueuedTurns: map, + runId: " run-a ", + controller: spaced, + sessionId: "sess-a", + sessionKey: "main", + }), + ).toBe(true); + expect( + registerQueuedChatTurn({ + chatQueuedTurns: map, + runId: "run-a", + controller: plain, + sessionId: "sess-a", + sessionKey: "main", + }), + ).toBe(true); + + expect(getQueuedChatTurn(map, " run-a ")?.controller).toBe(spaced); + expect(abortQueuedChatTurnById(map, { runId: " run-a ", sessionKey: "main" }).aborted).toBe( + true, + ); + expect(map.has("run-a")).toBe(true); + expect(plain.signal.aborted).toBe(false); + }); + + it("aborts by runId and removes the entry", () => { + const map = emptyMap(); + const controller = new AbortController(); + registerQueuedChatTurn({ + chatQueuedTurns: map, + runId: "run-b", + controller, + sessionId: "sess-b", + sessionKey: "main", + }); + const res = abortQueuedChatTurnById(map, { + runId: "run-b", + sessionKey: "main", + stopReason: "rpc", + }); + expect(res.aborted).toBe(true); + expect(controller.signal.aborted).toBe(true); + expect(map.has("run-b")).toBe(false); + }); + + it("retains a retired collect source identity until aggregate completion", () => { + const map = emptyMap(); + const controller = new AbortController(); + registerQueuedChatTurn({ + chatQueuedTurns: map, + runId: "run-collected", + controller, + sessionId: "sess-collected", + sessionKey: "main", + }); + + expect(retireQueuedChatTurnCancellation(map, "run-collected")).toBe(true); + expect( + abortQueuedChatTurnById(map, { runId: "run-collected", sessionKey: "main" }).aborted, + ).toBe(false); + expect(controller.signal.aborted).toBe(false); + expect(map.has("run-collected")).toBe(true); + expect(listQueuedChatTurnsForSession({ chatQueuedTurns: map, sessionKeys: ["main"] })).toEqual( + [], + ); + expect(completeQueuedChatTurn(map, "run-collected")).toBe(true); + }); + + it("refuses abort when sessionKey mismatches unless allowed", () => { + const map = emptyMap(); + const controller = new AbortController(); + registerQueuedChatTurn({ + chatQueuedTurns: map, + runId: "run-c", + controller, + sessionId: "sess-c", + sessionKey: "main", + }); + expect(abortQueuedChatTurnById(map, { runId: "run-c", sessionKey: "other" }).aborted).toBe( + false, + ); + expect(controller.signal.aborted).toBe(false); + expect( + abortQueuedChatTurnById(map, { + runId: "run-c", + sessionKey: "other", + allowSessionMismatch: true, + }).aborted, + ).toBe(true); + }); + + it("lists session matches and supports global agent scoping", () => { + const map = emptyMap(); + registerQueuedChatTurn({ + chatQueuedTurns: map, + runId: "g-main", + controller: new AbortController(), + sessionId: "s1", + sessionKey: "global", + agentId: "main", + }); + registerQueuedChatTurn({ + chatQueuedTurns: map, + runId: "g-other", + controller: new AbortController(), + sessionId: "s2", + sessionKey: "global", + agentId: "other", + }); + registerQueuedChatTurn({ + chatQueuedTurns: map, + runId: "local", + controller: new AbortController(), + sessionId: "s3", + sessionKey: "agent:main:main", + }); + const globalMain = listQueuedChatTurnsForSession({ + chatQueuedTurns: map, + sessionKeys: ["global"], + agentId: "main", + defaultAgentId: "main", + }); + expect(globalMain.map((m) => m.runId).toSorted()).toEqual(["g-main"]); + const local = listQueuedChatTurnsForSession({ + chatQueuedTurns: map, + sessionKeys: ["agent:main:main"], + }); + expect(local.map((m) => m.runId)).toEqual(["local"]); + }); + + it("aborts authorized matches before returning runIds", () => { + const map = emptyMap(); + const a = new AbortController(); + const b = new AbortController(); + registerQueuedChatTurn({ + chatQueuedTurns: map, + runId: "qa", + controller: a, + sessionId: "s", + sessionKey: "main", + }); + registerQueuedChatTurn({ + chatQueuedTurns: map, + runId: "qb", + controller: b, + sessionId: "s", + sessionKey: "main", + }); + const matches = listQueuedChatTurnsForSession({ + chatQueuedTurns: map, + sessionKeys: ["main"], + }); + const runIds = abortQueuedChatTurns(map, matches, "rpc"); + expect(runIds.toSorted()).toEqual(["qa", "qb"]); + expect(a.signal.aborted).toBe(true); + expect(b.signal.aborted).toBe(true); + expect(map.size).toBe(0); + }); +}); diff --git a/src/gateway/chat-queued-turns.ts b/src/gateway/chat-queued-turns.ts new file mode 100644 index 00000000000..ff7531fc7e4 --- /dev/null +++ b/src/gateway/chat-queued-turns.ts @@ -0,0 +1,214 @@ +/** + * Gateway-owned cancel identity for turns that have been admitted to the + * followup/collect queue but are not yet (or no longer) active chat-send runs. + * + * Active runs stay in chatAbortControllers. Queued waits must NOT look like + * active runs (projection, timeout ownership, terminal dedupe), but they must + * remain abortable by authorized requesters after chat.send terminalizes. + */ +import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; + +export type QueuedChatTurnEntry = { + controller: AbortController; + sessionId: string; + sessionKey: string; + /** False once collect-mode transfers cancellation to the aggregate owner. */ + abortable?: boolean; + agentId?: string; + ownerConnId?: string; + ownerDeviceId?: string; +}; + +export type QueuedChatTurnMap = Map; + +export type RegisterQueuedChatTurnParams = { + chatQueuedTurns: QueuedChatTurnMap; + runId: string; + controller: AbortController; + sessionId: string; + sessionKey: string; + agentId?: string; + ownerConnId?: string; + ownerDeviceId?: string; +}; + +function resolveExactRunId(runId: string): string | undefined { + // chat.send idempotency keys are exact protocol identities. Trimming here + // would diverge from the active-run and dedupe registries. + return runId.length > 0 ? runId : undefined; +} + +export function registerQueuedChatTurn(params: RegisterQueuedChatTurnParams): boolean { + const runId = resolveExactRunId(params.runId); + const sessionKey = normalizeOptionalString(params.sessionKey); + if (!runId || !sessionKey) { + return false; + } + if (params.controller.signal.aborted) { + return false; + } + const existing = params.chatQueuedTurns.get(runId); + if (existing && existing.controller === params.controller) { + return true; + } + if (existing) { + return false; + } + const entry: QueuedChatTurnEntry = { + controller: params.controller, + sessionId: params.sessionId, + sessionKey, + agentId: normalizeOptionalString(params.agentId)?.toLowerCase(), + ownerConnId: normalizeOptionalString(params.ownerConnId), + ownerDeviceId: normalizeOptionalString(params.ownerDeviceId), + }; + params.chatQueuedTurns.set(runId, entry); + return true; +} + +export function completeQueuedChatTurn(chatQueuedTurns: QueuedChatTurnMap, runId: string): boolean { + const key = resolveExactRunId(runId); + if (!key) { + return false; + } + return chatQueuedTurns.delete(key); +} + +/** + * Retain the live run identity for idempotency while transferring cancellation + * to a collect aggregate. Completion still removes the entry. + */ +export function retireQueuedChatTurnCancellation( + chatQueuedTurns: QueuedChatTurnMap, + runId: string, +): boolean { + const entry = getQueuedChatTurn(chatQueuedTurns, runId); + if (!entry) { + return false; + } + entry.abortable = false; + return true; +} + +export function getQueuedChatTurn( + chatQueuedTurns: QueuedChatTurnMap, + runId: string, +): QueuedChatTurnEntry | undefined { + const key = resolveExactRunId(runId); + if (!key) { + return undefined; + } + return chatQueuedTurns.get(key); +} + +/** + * Abort a single queued turn by runId. Does not authorize; caller must check. + * Returns false when missing or already aborted/removed. + */ +export function abortQueuedChatTurnById( + chatQueuedTurns: QueuedChatTurnMap, + params: { + runId: string; + sessionKey: string; + stopReason?: string; + /** When true, allow abort even if sessionKey does not match (owner already authorized). */ + allowSessionMismatch?: boolean; + }, +): { aborted: boolean } { + const runId = resolveExactRunId(params.runId); + const sessionKey = normalizeOptionalString(params.sessionKey); + if (!runId || !sessionKey) { + return { aborted: false }; + } + const entry = chatQueuedTurns.get(runId); + if (!entry || entry.abortable === false) { + return { aborted: false }; + } + if (!params.allowSessionMismatch && entry.sessionKey !== sessionKey) { + return { aborted: false }; + } + if (!entry.controller.signal.aborted) { + entry.controller.abort( + params.stopReason ? new Error(`queued turn aborted: ${params.stopReason}`) : undefined, + ); + } + chatQueuedTurns.delete(runId); + return { aborted: true }; +} + +export type QueuedChatTurnMatch = { + runId: string; + entry: QueuedChatTurnEntry; +}; + +/** + * List queued turns matching session keys / session ids / optional agent scope. + * Authorization is left to the caller. + */ +export function listQueuedChatTurnsForSession(params: { + chatQueuedTurns: QueuedChatTurnMap; + sessionKeys: Iterable; + sessionIds?: Iterable; + agentId?: string; + defaultAgentId?: string; +}): QueuedChatTurnMatch[] { + const sessionKeys = new Set( + Array.from(params.sessionKeys, (k) => normalizeOptionalString(k)).filter((k): k is string => + Boolean(k), + ), + ); + const sessionIds = new Set( + Array.from(params.sessionIds ?? [], (id) => normalizeOptionalString(id)).filter( + (id): id is string => Boolean(id), + ), + ); + const agentId = normalizeOptionalString(params.agentId)?.toLowerCase(); + const defaultAgentId = normalizeOptionalString(params.defaultAgentId)?.toLowerCase(); + const matches: QueuedChatTurnMatch[] = []; + for (const [runId, entry] of params.chatQueuedTurns) { + if (entry.abortable === false) { + continue; + } + if (!sessionKeys.has(entry.sessionKey) && !sessionIds.has(entry.sessionId)) { + continue; + } + if (agentId && entry.sessionKey === "global") { + const entryAgent = (entry.agentId ?? defaultAgentId)?.toLowerCase(); + if (entryAgent !== agentId) { + continue; + } + } + matches.push({ runId, entry }); + } + return matches; +} + +/** + * Abort all provided queued turns (already authorized by caller). + * Order: abort signals first, then remove from map, so drain cannot promote mid-loop. + */ +export function abortQueuedChatTurns( + chatQueuedTurns: QueuedChatTurnMap, + matches: readonly QueuedChatTurnMatch[], + stopReason?: string, +): string[] { + const runIds: string[] = []; + for (const { runId, entry } of matches) { + if (!chatQueuedTurns.has(runId)) { + continue; + } + if (!entry.controller.signal.aborted) { + entry.controller.abort( + stopReason ? new Error(`queued turn aborted: ${stopReason}`) : undefined, + ); + } + chatQueuedTurns.delete(runId); + runIds.push(runId); + } + return runIds; +} + +/** Test helper: clear all queued turns. */ +export function clearAllQueuedChatTurns(chatQueuedTurns: QueuedChatTurnMap): void { + chatQueuedTurns.clear(); +} diff --git a/src/gateway/local-request-context.ts b/src/gateway/local-request-context.ts index e4249588f61..b34fde52ef6 100644 --- a/src/gateway/local-request-context.ts +++ b/src/gateway/local-request-context.ts @@ -97,6 +97,7 @@ export function createLocalGatewayRequestContext( nodeRegistry: new NodeRegistry(), agentRunSeq: new Map(), chatAbortControllers: new Map(), + chatQueuedTurns: new Map(), chatAbortedRuns: new Map(), chatRunBuffers, chatDeltaSentAt, diff --git a/src/gateway/server-close.test.ts b/src/gateway/server-close.test.ts index 28c7e73c2e4..ef2b4df3c49 100644 --- a/src/gateway/server-close.test.ts +++ b/src/gateway/server-close.test.ts @@ -122,6 +122,7 @@ function createGatewayCloseTestDeps( lifecycleUnsub: null, chatRunState: createTestChatRunState(), chatAbortControllers: new Map(), + chatQueuedTurns: new Map(), restartRecoveryCandidates: new Map(), removeChatRun: vi.fn(), agentRunSeq: new Map(), @@ -616,6 +617,40 @@ describe("createGatewayCloseHandler", () => { ); }); + it("aborts queued turns before restart shutdown continues", async () => { + const controller = new AbortController(); + const chatQueuedTurns = new Map([ + [ + "queued-1", + { + controller, + sessionId: "session-1", + sessionKey: "session-1", + }, + ], + ]); + const close = createGatewayCloseHandler( + createGatewayCloseTestDeps({ + chatQueuedTurns, + }), + ); + + const result = await close({ + reason: "gateway restarting", + restartExpectedMs: 123, + drainTimeoutMs: 0, + }); + + expect(result.warnings).toContain("restart-reply-drain"); + expect(controller.signal.aborted).toBe(true); + expect(chatQueuedTurns.size).toBe(0); + expect( + mocks.logWarn.mock.calls.some(([message]) => + String(message).includes("aborted 1 queued turn(s) during restart shutdown"), + ), + ).toBe(true); + }); + it("does not drain or abort active runs for normal shutdown", async () => { const controller = new AbortController(); const chatAbortControllers = new Map([ diff --git a/src/gateway/server-close.ts b/src/gateway/server-close.ts index be29b4decff..0be94bcc0d4 100644 --- a/src/gateway/server-close.ts +++ b/src/gateway/server-close.ts @@ -19,6 +19,7 @@ import { removeChatAbortControllerEntry, type RestartRecoveryCandidate, } from "./chat-abort.js"; +import { abortQueuedChatTurns, type QueuedChatTurnMap } from "./chat-queued-turns.js"; import { collectGatewayProcessMemoryUsageMb, measureGatewayRestartTrace, @@ -108,15 +109,21 @@ function recordShutdownWarning(warnings: string[], name: string): void { function getRestartReplyDrainCounts(params: { getPendingReplyCount: () => number; chatAbortControllers: Map; + chatQueuedTurns: QueuedChatTurnMap; }) { const pendingReplyCount = params.getPendingReplyCount(); const activeRuns = listRestartDrainRuns(params.chatAbortControllers).length; + const queuedTurns = Array.from( + params.chatQueuedTurns.values(), + (entry) => entry.controller.signal.aborted, + ).filter((aborted) => !aborted).length; return { pendingReplies: Number.isFinite(pendingReplyCount) && pendingReplyCount > 0 ? Math.floor(pendingReplyCount) : 0, activeRuns, + queuedTurns, }; } @@ -154,6 +161,7 @@ function listRestartRecoveryRuns( function formatRestartReplyDrainDetails(counts: { pendingReplies: number; activeRuns: number; + queuedTurns: number; }): string { const details: string[] = []; if (counts.pendingReplies > 0) { @@ -162,6 +170,9 @@ function formatRestartReplyDrainDetails(counts: { if (counts.activeRuns > 0) { details.push(`${counts.activeRuns} active run(s)`); } + if (counts.queuedTurns > 0) { + details.push(`${counts.queuedTurns} queued turn(s)`); + } return details.length > 0 ? details.join(", ") : "no pending reply work"; } @@ -175,6 +186,7 @@ async function sleepForRestartReplyDrain(delayMs: number): Promise { type RestartRunAbortParams = { chatAbortControllers: Map; + chatQueuedTurns: QueuedChatTurnMap; restartRecoveryCandidates?: Map; chatRunState: ChatRunState; removeChatRun: ( @@ -211,17 +223,18 @@ type RestartRunAbortParams = { async function waitForRestartReplyDrain(params: { getPendingReplyCount: () => number; chatAbortControllers: Map; + chatQueuedTurns: QueuedChatTurnMap; timeoutMs: number; pollMs?: number; }): Promise<{ drained: boolean; elapsedMs: number; - counts: { pendingReplies: number; activeRuns: number }; + counts: { pendingReplies: number; activeRuns: number; queuedTurns: number }; }> { const timeoutMs = Math.max(0, Math.floor(params.timeoutMs)); const pollMs = Math.max(25, Math.floor(params.pollMs ?? RESTART_REPLY_DRAIN_POLL_MS)); let counts = getRestartReplyDrainCounts(params); - if (counts.pendingReplies <= 0 && counts.activeRuns <= 0) { + if (counts.pendingReplies <= 0 && counts.activeRuns <= 0 && counts.queuedTurns <= 0) { return { drained: true, elapsedMs: 0, counts }; } if (timeoutMs <= 0) { @@ -236,7 +249,7 @@ async function waitForRestartReplyDrain(params: { } await sleepForRestartReplyDrain(Math.min(pollMs, timeoutMs - elapsedMs)); counts = getRestartReplyDrainCounts(params); - if (counts.pendingReplies <= 0 && counts.activeRuns <= 0) { + if (counts.pendingReplies <= 0 && counts.activeRuns <= 0 && counts.queuedTurns <= 0) { return { drained: true, elapsedMs: Date.now() - startedAt, counts }; } } @@ -440,6 +453,12 @@ function abortActiveRunsForRestart(params: RestartRunAbortParams): number { return aborted; } +/** Abort queued owners before active teardown can promote them into the closing runtime. */ +function abortQueuedTurnsForRestart(params: RestartRunAbortParams): number { + const matches = Array.from(params.chatQueuedTurns, ([runId, entry]) => ({ runId, entry })); + return abortQueuedChatTurns(params.chatQueuedTurns, matches, "restart").length; +} + /** Drain or abort pending reply work before restart shutdown proceeds. */ async function drainRestartPendingRepliesForShutdown( params: { @@ -449,7 +468,12 @@ async function drainRestartPendingRepliesForShutdown( } & RestartRunAbortParams, ): Promise { const initialCounts = getRestartReplyDrainCounts(params); - if (initialCounts.pendingReplies <= 0 && initialCounts.activeRuns <= 0) { + if ( + initialCounts.pendingReplies <= 0 && + initialCounts.activeRuns <= 0 && + initialCounts.queuedTurns <= 0 + ) { + abortQueuedTurnsForRestart(params); await markActiveRunsForRestartRecovery({ ...params, reason: "gateway restart shutdown", @@ -468,9 +492,11 @@ async function drainRestartPendingRepliesForShutdown( const drainResult = await waitForRestartReplyDrain({ getPendingReplyCount: params.getPendingReplyCount, chatAbortControllers: params.chatAbortControllers, + chatQueuedTurns: params.chatQueuedTurns, timeoutMs, }); if (drainResult.drained) { + abortQueuedTurnsForRestart(params); await markActiveRunsForRestartRecovery({ ...params, reason: "gateway restart shutdown", @@ -485,6 +511,11 @@ async function drainRestartPendingRepliesForShutdown( ); recordShutdownWarning(params.warnings, "restart-reply-drain"); + const abortedQueuedTurns = abortQueuedTurnsForRestart(params); + if (abortedQueuedTurns > 0) { + shutdownLog.warn(`aborted ${abortedQueuedTurns} queued turn(s) during restart shutdown`); + } + if ( drainResult.counts.activeRuns <= 0 && (params.restartRecoveryCandidates?.size ?? 0) === 0 && @@ -506,6 +537,7 @@ async function drainRestartPendingRepliesForShutdown( const postAbortDrain = await waitForRestartReplyDrain({ getPendingReplyCount: params.getPendingReplyCount, chatAbortControllers: params.chatAbortControllers, + chatQueuedTurns: params.chatQueuedTurns, timeoutMs: RESTART_REPLY_POST_ABORT_DRAIN_TIMEOUT_MS, pollMs: RESTART_REPLY_POST_ABORT_DRAIN_POLL_MS, }); @@ -753,6 +785,7 @@ export function createGatewayCloseHandler( drainRestartPendingRepliesForShutdown({ getPendingReplyCount: params.getPendingReplyCount!, chatAbortControllers: params.chatAbortControllers, + chatQueuedTurns: params.chatQueuedTurns, restartRecoveryCandidates: params.restartRecoveryCandidates, chatRunState: params.chatRunState, removeChatRun: params.removeChatRun, diff --git a/src/gateway/server-maintenance.test.ts b/src/gateway/server-maintenance.test.ts index 9e08b3210b2..be96ca682b8 100644 --- a/src/gateway/server-maintenance.test.ts +++ b/src/gateway/server-maintenance.test.ts @@ -535,6 +535,52 @@ describe("startGatewayMaintenanceTimers", () => { stopMaintenanceTimers(timers); }); + it("keeps queued chat dedupe entries past the normal ttl", async () => { + const { startGatewayMaintenanceTimers, deps, now } = await createTimedMaintenanceScenario(); + const runId = "queued-chat"; + deps.chatQueuedTurns.set(runId, { + controller: new AbortController(), + sessionId: "session-main", + sessionKey: "agent:main:main", + }); + deps.dedupe.set(`chat:${runId}`, { + ts: now - DEDUPE_TTL_MS - 1, + ok: true, + payload: { runId, status: "ok" }, + }); + + const timers = startGatewayMaintenanceTimers(deps); + await vi.advanceTimersByTimeAsync(60_000); + + expect(deps.dedupe.has(`chat:${runId}`)).toBe(true); + stopMaintenanceTimers(timers); + }); + + it("keeps queued chat dedupe entries while trimming overflow", async () => { + const { startGatewayMaintenanceTimers, deps, now } = await createTimedMaintenanceScenario(); + const runId = "queued-oldest"; + seedStableDedupeEntries(deps, now); + deps.chatQueuedTurns.set(runId, { + controller: new AbortController(), + sessionId: "session-main", + sessionKey: "agent:main:main", + }); + deps.dedupe.set(`chat:${runId}`, { + ts: now - 10_000, + ok: true, + payload: { runId, status: "ok" }, + }); + deps.dedupe.set("overflow-newest", { ts: now, ok: true }); + + const timers = startGatewayMaintenanceTimers(deps); + await vi.advanceTimersByTimeAsync(60_000); + + expect(deps.dedupe.size).toBe(DEDUPE_MAX); + expect(deps.dedupe.has(`chat:${runId}`)).toBe(true); + expect(deps.dedupe.has("stable-0")).toBe(false); + stopMaintenanceTimers(timers); + }); + it("evicts dedupe overflow by oldest timestamp even after reinsertion", async () => { const { startGatewayMaintenanceTimers, deps, now } = await createTimedMaintenanceScenario(); diff --git a/src/gateway/server-maintenance.ts b/src/gateway/server-maintenance.ts index 51697be27d0..5dff38e063f 100644 --- a/src/gateway/server-maintenance.ts +++ b/src/gateway/server-maintenance.ts @@ -10,6 +10,7 @@ import { removeChatAbortControllerEntry, type RestartRecoveryCandidate, } from "./chat-abort.js"; +import type { QueuedChatTurnMap } from "./chat-queued-turns.js"; import { pruneStaleControlPlaneBuckets } from "./control-plane-rate-limit.js"; import { chatAbortMarkerTimestampMs } from "./server-chat-state.js"; import type { ChatRunState } from "./server-chat-state.js"; @@ -43,6 +44,7 @@ export function startGatewayMaintenanceTimers(params: { logHealth: { error: (msg: string) => void }; dedupe: Map; chatAbortControllers: Map; + chatQueuedTurns: QueuedChatTurnMap; restartRecoveryCandidates: Map; chatRunState: Pick< ChatRunState, @@ -110,8 +112,7 @@ export function startGatewayMaintenanceTimers(params: { } const keyRunId = key.slice(key.indexOf(":") + 1); if (keyRunId) { - const directEntry = params.chatAbortControllers.get(keyRunId); - if (directEntry) { + if (params.chatAbortControllers.has(keyRunId) || params.chatQueuedTurns.has(keyRunId)) { return keyRunId; } } @@ -146,10 +147,10 @@ export function startGatewayMaintenanceTimers(params: { } const runId = resolveDedupeRunId(key, dedupeEntry); const entry = runId ? params.chatAbortControllers.get(runId) : undefined; - if (!entry) { - return false; + if (entry) { + return isAgentKey ? entry.kind === "agent" : entry.kind !== "agent"; } - return isAgentKey ? entry.kind === "agent" : entry.kind !== "agent"; + return Boolean(isChatKey && runId && params.chatQueuedTurns.has(runId)); }; for (const [k, v] of params.dedupe) { if (isActiveRunDedupeKey(k, v) || isPendingAcceptedRunDedupeKey(k, v)) { diff --git a/src/gateway/server-methods/agent.test.ts b/src/gateway/server-methods/agent.test.ts index 1c92730ca9a..87aa4b39951 100644 --- a/src/gateway/server-methods/agent.test.ts +++ b/src/gateway/server-methods/agent.test.ts @@ -259,6 +259,7 @@ const makeContext = (): GatewayRequestContext => addChatRun: vi.fn(), removeChatRun: vi.fn(), chatAbortControllers: new Map(), + chatQueuedTurns: new Map(), chatRunBuffers: new Map(), chatDeltaSentAt: new Map(), chatDeltaLastBroadcastLen: new Map(), diff --git a/src/gateway/server-methods/chat.abort-authorization.test.ts b/src/gateway/server-methods/chat.abort-authorization.test.ts index 1e8daf12620..873126dc8fc 100644 --- a/src/gateway/server-methods/chat.abort-authorization.test.ts +++ b/src/gateway/server-methods/chat.abort-authorization.test.ts @@ -17,21 +17,29 @@ type AbortRespond = Awaited>; async function invokeAbort({ context, + sessionKey = "main", runId, connId, deviceId, + preserveSideRuns, scopes = ["operator.write"], }: { context: ReturnType; + sessionKey?: string; runId?: string; connId: string; deviceId: string; + preserveSideRuns?: boolean; scopes?: string[]; }) { return await invokeChatAbortHandler({ handler: chatHandlers["chat.abort"], context, - request: { sessionKey: "main", ...(runId ? { runId } : {}) }, + request: { + sessionKey, + ...(runId ? { runId } : {}), + ...(preserveSideRuns ? { preserveSideRuns: true } : {}), + }, client: { connId, connect: { device: { id: deviceId }, scopes }, @@ -127,6 +135,68 @@ describe("chat.abort authorization", () => { expect(context.chatAbortControllers.has("run-hidden")).toBe(true); }); + it("preserves BTW runs for TUI session stops", async () => { + const main = createActiveRun("main", { + owner: { connId: "conn-owner", deviceId: "dev-owner" }, + }); + const btw = createActiveRun("main", { + owner: { connId: "conn-owner", deviceId: "dev-owner" }, + turnKind: "btw", + }); + const context = createChatAbortContext({ + chatAbortControllers: new Map([ + ["run-main", main], + ["run-btw", btw], + ]), + }); + + const respond = await invokeAbort({ + context, + connId: "conn-owner", + deviceId: "dev-owner", + preserveSideRuns: true, + }); + + const [ok, payload] = requireLastRespondCall(respond); + expect(ok).toBe(true); + expectAbortPayload(payload, { aborted: true, runIds: ["run-main"] }); + expect(main.controller.signal.aborted).toBe(true); + expect(btw.controller.signal.aborted).toBe(false); + expect(context.chatAbortControllers.has("run-btw")).toBe(true); + }); + + it("preserves BTW runs waiting for chat admission", async () => { + const context = createChatAbortContext(); + context.dedupe.set("pending-chat:run-btw", { + ts: Date.now(), + ok: true, + payload: { + runId: "run-btw", + sessionKey: "main", + status: "accepted", + turnKind: "btw", + ownerConnId: "conn-owner", + ownerDeviceId: "dev-owner", + }, + }); + + const respond = await invokeAbort({ + context, + connId: "conn-owner", + deviceId: "dev-owner", + preserveSideRuns: true, + }); + + const [ok, payload] = requireLastRespondCall(respond); + expect(ok).toBe(true); + expectAbortPayload(payload, { aborted: false, runIds: [] }); + expect(context.dedupe.get("pending-chat:run-btw")).toEqual( + expect.objectContaining({ + payload: expect.objectContaining({ status: "accepted", turnKind: "btw" }), + }), + ); + }); + it("clears agent text throttle state through the real abort caller", async () => { const context = createChatAbortContext({ chatAbortControllers: new Map([ @@ -200,3 +270,167 @@ describe("chat.abort authorization", () => { expectAbortPayload(payload, { aborted: true, runIds: ["run-1"] }); }); }); + +describe("chat.abort queued-turn contract", () => { + it("aborts a queued turn by runId after active registration is gone", async () => { + const controller = new AbortController(); + const context = createChatAbortContext({ + chatQueuedTurns: new Map([ + [ + "queued-1", + { + controller, + sessionId: "main-session", + sessionKey: "main", + ownerConnId: "conn-owner", + ownerDeviceId: "dev-owner", + }, + ], + ]), + }); + + const respond = await invokeAbort({ + context, + runId: "queued-1", + connId: "conn-owner", + deviceId: "dev-owner", + }); + const call = requireLastRespondCall(respond); + expect(call[0]).toBe(true); + expectAbortPayload(call[1], { aborted: true, runIds: ["queued-1"] }); + expect(controller.signal.aborted).toBe(true); + expect(context.chatQueuedTurns.has("queued-1")).toBe(false); + }); + + it("rejects queued-turn abort from other clients", async () => { + const controller = new AbortController(); + const context = createChatAbortContext({ + chatQueuedTurns: new Map([ + [ + "queued-1", + { + controller, + sessionId: "main-session", + sessionKey: "main", + ownerConnId: "conn-owner", + ownerDeviceId: "dev-owner", + }, + ], + ]), + }); + + const respond = await invokeAbort({ + context, + runId: "queued-1", + connId: "conn-other", + deviceId: "dev-other", + }); + const call = requireLastRespondCall(respond); + expect(call[0]).toBe(false); + expect(controller.signal.aborted).toBe(false); + expect(context.chatQueuedTurns.has("queued-1")).toBe(true); + }); + + it("rejects a mismatched session for ownerless queued turns", async () => { + const controller = new AbortController(); + const context = createChatAbortContext({ + chatQueuedTurns: new Map([ + [ + "queued-ownerless", + { + controller, + sessionId: "main-session", + sessionKey: "main", + }, + ], + ]), + }); + + const respond = await invokeAbort({ + context, + sessionKey: "other", + runId: "queued-ownerless", + connId: "conn-other", + deviceId: "dev-other", + }); + const call = requireLastRespondCall(respond); + expect(call[0]).toBe(false); + expect(call[2]?.message).toBe("runId does not match sessionKey"); + expect(controller.signal.aborted).toBe(false); + expect(context.chatQueuedTurns.has("queued-ownerless")).toBe(true); + }); + + it("session abort cancels authorized queued turns before active runs", async () => { + const queuedController = new AbortController(); + const activeController = new AbortController(); + const context = createChatAbortContext({ + chatAbortControllers: new Map([ + [ + "active-1", + createActiveRun("main", { owner: { connId: "conn-owner", deviceId: "dev-owner" } }), + ], + ]), + chatQueuedTurns: new Map([ + [ + "queued-1", + { + controller: queuedController, + sessionId: "main-session", + sessionKey: "main", + ownerConnId: "conn-owner", + ownerDeviceId: "dev-owner", + }, + ], + ]), + }); + // replace active controller so we can observe abort + const active = context.chatAbortControllers.get("active-1"); + if (active) { + (active as { controller: AbortController }).controller = activeController; + } + + const respond = await invokeAbort({ + context, + connId: "conn-owner", + deviceId: "dev-owner", + }); + const call = requireLastRespondCall(respond); + expect(call[0]).toBe(true); + const payload = call[1] as AbortResponsePayload; + expect(payload.aborted).toBe(true); + expect(payload.runIds).toEqual(expect.arrayContaining(["queued-1", "active-1"])); + expect(payload.runIds?.[0]).toBe("queued-1"); + expect(queuedController.signal.aborted).toBe(true); + expect(activeController.signal.aborted).toBe(true); + expect(context.chatQueuedTurns.size).toBe(0); + }); + + it("session abort does not clear another owner's queued turns", async () => { + const foreign = new AbortController(); + const context = createChatAbortContext({ + chatQueuedTurns: new Map([ + [ + "queued-foreign", + { + controller: foreign, + sessionId: "main-session", + sessionKey: "main", + ownerConnId: "conn-owner", + ownerDeviceId: "dev-owner", + }, + ], + ]), + }); + + const respond = await invokeAbort({ + context, + connId: "conn-other", + deviceId: "dev-other", + }); + const call = requireLastRespondCall(respond); + // unauthorized when only foreign queued matches + expect(call[0]).toBe(false); + expect(foreign.signal.aborted).toBe(false); + expect(context.chatQueuedTurns.has("queued-foreign")).toBe(true); + }); +}); diff --git a/src/gateway/server-methods/chat.abort.test-helpers.ts b/src/gateway/server-methods/chat.abort.test-helpers.ts index 73e97aad3a6..d9bd5751d73 100644 --- a/src/gateway/server-methods/chat.abort.test-helpers.ts +++ b/src/gateway/server-methods/chat.abort.test-helpers.ts @@ -13,6 +13,7 @@ export function createActiveRun( agentId?: string; controlUiVisible?: boolean; owner?: { connId?: string; deviceId?: string }; + turnKind?: "main" | "btw"; } = {}, ) { const now = Date.now(); @@ -26,11 +27,13 @@ export function createActiveRun( controlUiVisible: params.controlUiVisible, ownerConnId: params.owner?.connId, ownerDeviceId: params.owner?.deviceId, + turnKind: params.turnKind, }; } type ChatAbortTestContext = Record & { chatAbortControllers: Map>; + chatQueuedTurns: Map; chatRunBuffers: Map; chatDeltaSentAt: Map; chatDeltaLastBroadcastLen: Map; @@ -56,6 +59,7 @@ export function createChatAbortContext( ): ChatAbortTestContext { const context = { chatAbortControllers: new Map(), + chatQueuedTurns: new Map(), chatRunBuffers: new Map(), chatDeltaSentAt: new Map(), chatDeltaLastBroadcastLen: new Map(), @@ -93,7 +97,12 @@ export function createChatAbortContext( export async function invokeChatAbortHandler(params: { handler: GatewayRequestHandler; context: ChatAbortTestContext; - request: { sessionKey: string; agentId?: string; runId?: string }; + request: { + sessionKey: string; + agentId?: string; + runId?: string; + preserveSideRuns?: boolean; + }; client?: { connId?: string; connect?: { diff --git a/src/gateway/server-methods/chat.directive-tags.test.ts b/src/gateway/server-methods/chat.directive-tags.test.ts index 0872712fd3c..afa41d47ad5 100644 --- a/src/gateway/server-methods/chat.directive-tags.test.ts +++ b/src/gateway/server-methods/chat.directive-tags.test.ts @@ -674,6 +674,7 @@ function createChatContext(): Pick< | "nodeSendToSession" | "agentRunSeq" | "chatAbortControllers" + | "chatQueuedTurns" | "chatRunBuffers" | "chatDeltaSentAt" | "chatDeltaLastBroadcastLen" @@ -697,6 +698,7 @@ function createChatContext(): Pick< nodeSendToSession: vi.fn() as unknown as GatewayRequestContext["nodeSendToSession"], agentRunSeq: new Map(), chatAbortControllers: new Map(), + chatQueuedTurns: new Map(), chatRunBuffers: new Map(), chatDeltaSentAt: new Map(), chatDeltaLastBroadcastLen: new Map(), diff --git a/src/gateway/server-methods/chat.ts b/src/gateway/server-methods/chat.ts index 17692c08232..dda91434a21 100644 --- a/src/gateway/server-methods/chat.ts +++ b/src/gateway/server-methods/chat.ts @@ -56,6 +56,7 @@ import { readPairingQrReplyChannelData, type ReplyPayload, } from "../../auto-reply/reply-payload.js"; +import { isBtwRequestText } from "../../auto-reply/reply/btw-command.js"; import { createReplyDispatcher } from "../../auto-reply/reply/reply-dispatcher.js"; import { isReplyRunAbortableForSignal } from "../../auto-reply/reply/reply-run-registry.js"; import { @@ -155,6 +156,16 @@ import { resolveEffectiveChatHistoryMaxChars, } from "../chat-display-projection.js"; import { sanitizeChatSendMessageInput } from "../chat-input-sanitize.js"; +import { + abortQueuedChatTurnById, + abortQueuedChatTurns, + completeQueuedChatTurn, + listQueuedChatTurnsForSession, + registerQueuedChatTurn, + retireQueuedChatTurnCancellation, + type QueuedChatTurnEntry, + type QueuedChatTurnMap, +} from "../chat-queued-turns.js"; import { stripEnvelopeFromMessage } from "../chat-sanitize.js"; import { augmentChatHistoryWithCliSessionImports } from "../cli-session-history.js"; import { isSuppressedControlReplyText } from "../control-reply-text.js"; @@ -260,6 +271,7 @@ type PreRegisteredAgentDedupePayload = { sessionKey?: unknown; sessionKeyAliases?: unknown; status?: unknown; + turnKind?: unknown; }; type PreRegisteredAgentRun = { @@ -2352,6 +2364,7 @@ function resolveAuthorizedPreRegisteredRunsForSessionKeys(params: { defaultAgentId: string; requester: ChatAbortRequester; keyPrefix: string; + preserveSideRuns?: boolean; }) { const sessionKeys = new Set( Array.from(params.sessionKeys, (sessionKey) => normalizeOptionalText(sessionKey)).filter( @@ -2365,6 +2378,9 @@ function resolveAuthorizedPreRegisteredRunsForSessionKeys(params: { if (!run) { continue; } + if (params.preserveSideRuns && normalizeUnknownText(run.payload.turnKind) === "btw") { + continue; + } const runSessionKeys = [ run.sessionKey, ...(Array.isArray(run.payload.sessionKeyAliases) @@ -2406,6 +2422,7 @@ function resolveAuthorizedRunsForSessionKeys(params: { agentId?: string; defaultAgentId: string; requester: ChatAbortRequester; + preserveSideRuns?: boolean; }) { const sessionKeys = new Set( Array.from(params.sessionKeys, (sessionKey) => normalizeOptionalText(sessionKey)).filter( @@ -2424,6 +2441,9 @@ function resolveAuthorizedRunsForSessionKeys(params: { if (active.controlUiVisible === false) { continue; } + if (params.preserveSideRuns && active.turnKind === "btw") { + continue; + } if (!sessionKeys.has(active.sessionKey) && !sessionIds.has(active.sessionId)) { continue; } @@ -2445,6 +2465,81 @@ function resolveAuthorizedRunsForSessionKeys(params: { }; } +function ensureChatQueuedTurns(context: GatewayRequestContext): QueuedChatTurnMap { + return context.chatQueuedTurns; +} + +function canRequesterAbortQueuedChatTurn( + entry: QueuedChatTurnEntry, + requester: ChatAbortRequester, +): boolean { + // Same ownership rules as active chat runs. + if (requester.isAdmin) { + return true; + } + const ownerDeviceId = normalizeOptionalText(entry.ownerDeviceId); + const ownerConnId = normalizeOptionalText(entry.ownerConnId); + if (!ownerDeviceId && !ownerConnId) { + return true; + } + if (ownerDeviceId && requester.deviceId && ownerDeviceId === requester.deviceId) { + return true; + } + if (ownerConnId && requester.connId && ownerConnId === requester.connId) { + return true; + } + return false; +} + +function canRequesterAbortQueuedChatTurnWithoutSessionMatch( + entry: QueuedChatTurnEntry, + requester: ChatAbortRequester, +): boolean { + if (requester.isAdmin) { + return true; + } + const ownerDeviceId = normalizeOptionalText(entry.ownerDeviceId); + const ownerConnId = normalizeOptionalText(entry.ownerConnId); + return Boolean( + (ownerDeviceId && requester.deviceId && ownerDeviceId === requester.deviceId) || + (ownerConnId && requester.connId && ownerConnId === requester.connId), + ); +} + +/** + * Cancel authorized queued turns for a session BEFORE active-run abort so + * drain cannot promote work into a half-aborted session. + */ +function abortAuthorizedQueuedTurnsForSession(params: { + context: GatewayRequestContext; + sessionKeys: string[]; + sessionId?: string; + agentId?: string; + defaultAgentId: string; + requester: ChatAbortRequester; + stopReason?: string; +}): { runIds: string[]; matched: number; unauthorizedOnly: boolean } { + const chatQueuedTurns = ensureChatQueuedTurns(params.context); + const matches = listQueuedChatTurnsForSession({ + chatQueuedTurns, + sessionKeys: params.sessionKeys, + sessionIds: [params.sessionId], + agentId: params.agentId, + defaultAgentId: params.defaultAgentId, + }); + if (matches.length === 0) { + return { runIds: [], matched: 0, unauthorizedOnly: false }; + } + const authorized = matches.filter((m) => + canRequesterAbortQueuedChatTurn(m.entry, params.requester), + ); + if (authorized.length === 0) { + return { runIds: [], matched: matches.length, unauthorizedOnly: true }; + } + const runIds = abortQueuedChatTurns(chatQueuedTurns, authorized, params.stopReason); + return { runIds, matched: matches.length, unauthorizedOnly: false }; +} + async function abortChatRunsForSessionKeyWithPartials(params: { context: GatewayRequestContext; ops: ChatAbortOps; @@ -2457,8 +2552,20 @@ async function abortChatRunsForSessionKeyWithPartials(params: { abortOrigin: AbortOrigin; stopReason?: string; requester: ChatAbortRequester; + preserveSideRuns?: boolean; }): Promise<{ aborted: boolean; runIds: string[]; unauthorized: boolean }> { const sessionKeys = [params.sessionKey, ...(params.sessionKeyAliases ?? [])]; + // Queued-turn cancel MUST run before active abort so followup drain cannot + // promote cancelled work into the gap between active stop and queue clear. + const queuedAbort = abortAuthorizedQueuedTurnsForSession({ + context: params.context, + sessionKeys, + sessionId: params.sessionId, + agentId: params.agentId, + defaultAgentId: params.defaultAgentId, + requester: params.requester, + stopReason: params.stopReason, + }); const { matchedSessionRuns, authorizedRuns } = resolveAuthorizedRunsForSessionKeys({ chatAbortControllers: params.context.chatAbortControllers, sessionKeys, @@ -2466,6 +2573,7 @@ async function abortChatRunsForSessionKeyWithPartials(params: { agentId: params.agentId, defaultAgentId: params.defaultAgentId, requester: params.requester, + preserveSideRuns: params.preserveSideRuns, }); const { matchedSessionRuns: matchedPendingAgentRuns, @@ -2477,6 +2585,7 @@ async function abortChatRunsForSessionKeyWithPartials(params: { defaultAgentId: params.defaultAgentId, requester: params.requester, keyPrefix: "agent:", + preserveSideRuns: params.preserveSideRuns, }); const { matchedSessionRuns: matchedPendingChatRuns, authorizedRuns: authorizedPendingChatRuns } = resolveAuthorizedPreRegisteredRunsForSessionKeys({ @@ -2486,17 +2595,22 @@ async function abortChatRunsForSessionKeyWithPartials(params: { defaultAgentId: params.defaultAgentId, requester: params.requester, keyPrefix: PENDING_CHAT_SEND_DEDUPE_PREFIX, + preserveSideRuns: params.preserveSideRuns, }); if ( authorizedRuns.length === 0 && authorizedPendingAgentRuns.length === 0 && - authorizedPendingChatRuns.length === 0 + authorizedPendingChatRuns.length === 0 && + queuedAbort.runIds.length === 0 ) { return { aborted: false, runIds: [], unauthorized: - matchedSessionRuns > 0 || matchedPendingAgentRuns > 0 || matchedPendingChatRuns > 0, + matchedSessionRuns > 0 || + matchedPendingAgentRuns > 0 || + matchedPendingChatRuns > 0 || + queuedAbort.unauthorizedOnly, }; } const authorizedRunIdSet = new Set(authorizedRuns.map((run) => run.runId)); @@ -2506,7 +2620,8 @@ async function abortChatRunsForSessionKeyWithPartials(params: { runIds: authorizedRunIdSet, abortOrigin: params.abortOrigin, }); - const runIds: string[] = []; + // Queued cancellations already applied above; keep them first in the response. + const runIds: string[] = [...queuedAbort.runIds]; for (const { runId, sessionKey } of authorizedRuns) { const res = abortChatRunById(params.ops, { runId, @@ -2541,7 +2656,7 @@ async function abortChatRunsForSessionKeyWithPartials(params: { runIds.push(runId); } const res = { aborted: runIds.length > 0, runIds, unauthorized: false }; - if (res.aborted) { + if (res.aborted && snapshots.length > 0) { const abortedRunIds = new Set(runIds); await persistAbortedPartials({ context: params.context, @@ -3268,10 +3383,15 @@ export const chatHandlers: GatewayRequestHandlers = { ); return; } - const { sessionKey: rawSessionKey, runId } = params as { + const { + sessionKey: rawSessionKey, + runId, + preserveSideRuns, + } = params as { sessionKey: string; agentId?: string; runId?: string; + preserveSideRuns?: boolean; }; const agentIdOverride = normalizeOptionalText((params as { agentId?: string }).agentId); const abortCfg = context.getRuntimeConfig(); @@ -3319,6 +3439,7 @@ export const chatHandlers: GatewayRequestHandlers = { abortOrigin: "rpc", stopReason: "rpc", requester, + preserveSideRuns, }); if (res.unauthorized) { respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unauthorized")); @@ -3393,6 +3514,53 @@ export const chatHandlers: GatewayRequestHandlers = { respond(true, { ok: true, aborted: true, runIds: [runId] }); return; } + // Queued followup/collect turns keep a cancel identity after chat.send + // terminalizes; abort them here so Esc cannot report done while they run. + const chatQueuedTurns = ensureChatQueuedTurns(context); + const queued = chatQueuedTurns.get(runId); + if (queued) { + const abortSessionKeysForQueued = new Set([rawSessionKey, canonicalAbortSessionKey]); + if ( + !abortSessionKeysForQueued.has(queued.sessionKey) && + !canRequesterAbortQueuedChatTurnWithoutSessionMatch(queued, requester) + ) { + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, "runId does not match sessionKey"), + ); + return; + } + if ( + normalizedAgentIdOverride && + queued.sessionKey === "global" && + resolveStoredGlobalRunAgentId(queued.agentId, defaultAgentId) !== + normalizedAgentIdOverride + ) { + respond( + false, + undefined, + errorShape(ErrorCodes.INVALID_REQUEST, "runId does not match agentId"), + ); + return; + } + if (!canRequesterAbortQueuedChatTurn(queued, requester)) { + respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "unauthorized")); + return; + } + const queuedRes = abortQueuedChatTurnById(chatQueuedTurns, { + runId, + sessionKey: queued.sessionKey, + stopReason: "rpc", + allowSessionMismatch: true, + }); + respond(true, { + ok: true, + aborted: queuedRes.aborted, + runIds: queuedRes.aborted ? [runId] : [], + }); + return; + } respond(true, { ok: true, aborted: false, runIds: [] }); return; } @@ -3540,6 +3708,8 @@ export const chatHandlers: GatewayRequestHandlers = { const systemInputProvenance = normalizeInputProvenance(p.systemInputProvenance); const systemProvenanceReceipt = systemReceiptResult.receipt; const stopCommand = !suppressCommandInterpretation && isChatStopCommandText(inboundMessage); + const turnKind = + !suppressCommandInterpretation && isBtwRequestText(inboundMessage) ? "btw" : "main"; const normalizedAttachments = normalizeRpcAttachmentsToChatAttachments(p.attachments); const rawMessage = inboundMessage.trim(); if (!rawMessage && normalizedAttachments.length === 0) { @@ -3720,6 +3890,13 @@ export const chatHandlers: GatewayRequestHandlers = { }); return; } + if (context.chatQueuedTurns?.has(clientRunId)) { + respond(true, { runId: clientRunId, status: "in_flight" as const }, undefined, { + cached: true, + runId: clientRunId, + }); + return; + } const archivedSessionError = resolveSessionWorkStartError(sessionKey, entry); if (archivedSessionError) { respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, archivedSessionError)); @@ -3764,6 +3941,7 @@ export const chatHandlers: GatewayRequestHandlers = { ownerConnId: normalizeOptionalText(client?.connId), ownerDeviceId: normalizeOptionalText(client?.connect?.device?.id), expiresAtMs: pendingExpiresAtMs, + turnKind, }, }); const clearPendingChatSendReservation = () => { @@ -3867,6 +4045,7 @@ export const chatHandlers: GatewayRequestHandlers = { authProviderId: resolvedSessionAuthProvider, isAbortable: (active) => isReplyRunAbortableForSignal(active.controller.signal), kind: "chat-send", + turnKind, lifecycleGeneration, }); }, @@ -4182,6 +4361,13 @@ export const chatHandlers: GatewayRequestHandlers = { const messageForAgent = systemProvenanceReceipt ? [systemProvenanceReceipt, parsedMessage].filter(Boolean).join("\n\n") : parsedMessage; + const queuedFollowupOwnerDeviceId = normalizeOptionalText(client?.connect?.device?.id); + const queuedFollowupOwnerConnId = normalizeOptionalText(client?.connId); + const queuedFollowupOwnerKey = queuedFollowupOwnerDeviceId + ? `device:${queuedFollowupOwnerDeviceId}` + : queuedFollowupOwnerConnId + ? `connection:${queuedFollowupOwnerConnId}` + : undefined; const { originatingChannel, originatingTo, @@ -4229,7 +4415,7 @@ export const chatHandlers: GatewayRequestHandlers = { body: commandBody, }, MessageSid: clientRunId, - ApprovalReviewerDeviceId: normalizeOptionalText(client?.connect?.device?.id), + ApprovalReviewerDeviceId: queuedFollowupOwnerDeviceId, ...(!isOperatorUiClient(clientInfo) ? { SenderId: clientInfo?.id, @@ -4271,6 +4457,7 @@ export const chatHandlers: GatewayRequestHandlers = { const deliveredReplies: Array<{ payload: ReplyPayload; kind: "block" | "final" }> = []; let appendedWebchatAgentMedia = false; let agentRunStarted = false; + let queuedFollowupEnqueued = false; let pendingDispatchLifecycleError: | { endedAt: number; @@ -4516,6 +4703,30 @@ export const chatHandlers: GatewayRequestHandlers = { requestedSessionId, resumeRequestedSession: controlUiReconnectResume.resumeRequested, abortSignal: activeRunAbort.controller.signal, + // Keep a Gateway-owned cancel identity after this chat.send + // terminalizes while the prompt waits in followup/collect queue. + queuedFollowupLifecycle: { + ownerKey: queuedFollowupOwnerKey, + onEnqueued: () => { + queuedFollowupEnqueued = registerQueuedChatTurn({ + chatQueuedTurns: ensureChatQueuedTurns(context), + runId: clientRunId, + controller: activeRunAbort.controller, + sessionId: backingSessionId ?? clientRunId, + sessionKey, + agentId: selectedAgent.agentId, + ownerConnId: normalizeOptionalText(client?.connId), + ownerDeviceId: normalizeOptionalText(client?.connect?.device?.id), + }); + return queuedFollowupEnqueued; + }, + onCancellationRetired: () => { + retireQueuedChatTurnCancellation(ensureChatQueuedTurns(context), clientRunId); + }, + onComplete: () => { + completeQueuedChatTurn(ensureChatQueuedTurns(context), clientRunId); + }, + }, images: replyOptionImages, imageOrder: imageOrder.length > 0 ? imageOrder : undefined, thinkingLevelOverride: p.thinking, @@ -4634,7 +4845,7 @@ export const chatHandlers: GatewayRequestHandlers = { // duplicate normal embedded-agent assistant turns. The non-agent branch below has no // runtime-owned assistant turn, so it appends a gateway-injected assistant entry before // broadcasting the final UI event. - if (!agentRunStarted) { + if (!agentRunStarted && !queuedFollowupEnqueued) { const btwReplies = deliveredReplies .map((entryScoped) => entryScoped.payload) .filter(isBtwReplyPayload); @@ -5499,9 +5710,42 @@ export const chatHandlers: GatewayRequestHandlers = { }, dispatchStartedAtMs, ); + if (queuedFollowupEnqueued && !context.chatAbortedRuns.has(clientRunId)) { + // Successful queue admission ends this client run. The later + // aggregate/followup owns its own run id. + broadcastChatFinal({ + context, + runId: clientRunId, + sessionKey, + agentId, + }); + } }) .catch(async (err: unknown) => { const errorMessage = String(err); + if (queuedFollowupEnqueued) { + context.logGateway.warn( + `webchat dispatch failed after followup queue admission: ${formatForLog(err)}`, + ); + if (!context.chatAbortedRuns.has(clientRunId)) { + setGatewayDedupeEntry({ + dedupe: context.dedupe, + key: `chat:${clientRunId}`, + entry: { + ts: Date.now(), + ok: true, + payload: { runId: clientRunId, status: "ok" as const }, + }, + }); + broadcastChatFinal({ + context, + runId: clientRunId, + sessionKey, + agentId, + }); + } + return; + } const emitAfterError = userTurnRecorder.hasPersisted() || userTurnRecorder.isBlocked() ? Promise.resolve() diff --git a/src/gateway/server-methods/shared-types.ts b/src/gateway/server-methods/shared-types.ts index 4124fb39bf3..a662b1af830 100644 --- a/src/gateway/server-methods/shared-types.ts +++ b/src/gateway/server-methods/shared-types.ts @@ -110,6 +110,8 @@ export type GatewayRequestContext = { terminalSessions?: TerminalSessionManager; agentRunSeq: Map; chatAbortControllers: Map; + /** Cancel identities for turns waiting in the followup/collect queue. */ + chatQueuedTurns: Map; chatAbortedRuns: Map; chatRunBuffers: Map; chatDeltaSentAt: Map; diff --git a/src/gateway/server-request-context.test.ts b/src/gateway/server-request-context.test.ts index 06068a454a4..9b246137286 100644 --- a/src/gateway/server-request-context.test.ts +++ b/src/gateway/server-request-context.test.ts @@ -49,6 +49,7 @@ function makeContextParams( nodeRegistry: {} as never, agentRunSeq: new Map(), chatAbortControllers: new Map(), + chatQueuedTurns: new Map(), chatAbortedRuns: new Map(), chatRunBuffers: new Map(), chatDeltaSentAt: new Map(), diff --git a/src/gateway/server-request-context.ts b/src/gateway/server-request-context.ts index 8313e2d1fef..9e10fb73b54 100644 --- a/src/gateway/server-request-context.ts +++ b/src/gateway/server-request-context.ts @@ -41,6 +41,7 @@ export type GatewayRequestContextParams = { terminalSessions?: GatewayRequestContext["terminalSessions"]; agentRunSeq: GatewayRequestContext["agentRunSeq"]; chatAbortControllers: GatewayRequestContext["chatAbortControllers"]; + chatQueuedTurns: GatewayRequestContext["chatQueuedTurns"]; chatAbortedRuns: GatewayRequestContext["chatAbortedRuns"]; chatRunBuffers: GatewayRequestContext["chatRunBuffers"]; chatDeltaSentAt: GatewayRequestContext["chatDeltaSentAt"]; @@ -185,6 +186,7 @@ export function createGatewayRequestContext( terminalSessions: params.terminalSessions, agentRunSeq: params.agentRunSeq, chatAbortControllers: params.chatAbortControllers, + chatQueuedTurns: params.chatQueuedTurns, chatAbortedRuns: params.chatAbortedRuns, chatRunBuffers: params.chatRunBuffers, chatDeltaSentAt: params.chatDeltaSentAt, diff --git a/src/gateway/server-runtime-state.ts b/src/gateway/server-runtime-state.ts index 4875f99dd99..4e5375378d7 100644 --- a/src/gateway/server-runtime-state.ts +++ b/src/gateway/server-runtime-state.ts @@ -125,6 +125,7 @@ export async function createGatewayRuntimeState(params: { sessionKey?: string, ) => ChatRunEntry | undefined; chatAbortControllers: Map; + chatQueuedTurns: Map; toolEventRecipients: ReturnType; }> { pinActivePluginHttpRouteRegistry(params.pluginRegistry); @@ -342,6 +343,7 @@ export async function createGatewayRuntimeState(params: { const addChatRun = chatRunRegistry.add; const removeChatRun = chatRunRegistry.remove; const chatAbortControllers = new Map(); + const chatQueuedTurns = new Map(); const toolEventRecipients = createToolEventRecipientRegistry(); return { @@ -374,6 +376,7 @@ export async function createGatewayRuntimeState(params: { addChatRun, removeChatRun, chatAbortControllers, + chatQueuedTurns, toolEventRecipients, }; } catch (err) { diff --git a/src/gateway/server-startup-early.ts b/src/gateway/server-startup-early.ts index 0e421fc606d..15bbdfba943 100644 --- a/src/gateway/server-startup-early.ts +++ b/src/gateway/server-startup-early.ts @@ -95,6 +95,7 @@ export async function startGatewayEarlyRuntime(params: { logHealth: GatewayMaintenanceParams["logHealth"]; dedupe: GatewayMaintenanceParams["dedupe"]; chatAbortControllers: GatewayMaintenanceParams["chatAbortControllers"]; + chatQueuedTurns: GatewayMaintenanceParams["chatQueuedTurns"]; restartRecoveryCandidates: GatewayMaintenanceParams["restartRecoveryCandidates"]; chatRunState: GatewayMaintenanceParams["chatRunState"]; chatRunBuffers: GatewayMaintenanceParams["chatRunBuffers"]; @@ -187,6 +188,7 @@ export async function startGatewayEarlyRuntime(params: { logHealth: params.logHealth, dedupe: params.dedupe, chatAbortControllers: params.chatAbortControllers, + chatQueuedTurns: params.chatQueuedTurns, restartRecoveryCandidates: params.restartRecoveryCandidates, chatRunState: params.chatRunState, chatRunBuffers: params.chatRunBuffers, diff --git a/src/gateway/server-startup-post-attach.test.ts b/src/gateway/server-startup-post-attach.test.ts index 23a047f10fa..648b46d454f 100644 --- a/src/gateway/server-startup-post-attach.test.ts +++ b/src/gateway/server-startup-post-attach.test.ts @@ -1918,6 +1918,7 @@ describe("startGatewayPostAttachRuntime", () => { lifecycleUnsub: null, chatRunState: createChatRunState(), chatAbortControllers: new Map(), + chatQueuedTurns: new Map(), removeChatRun: vi.fn(), agentRunSeq: new Map(), nodeSendToSession: vi.fn(), diff --git a/src/gateway/server.chat.gateway-server-chat-b.test.ts b/src/gateway/server.chat.gateway-server-chat-b.test.ts index 1ef4ec6f20e..c8d14fbb3a8 100644 --- a/src/gateway/server.chat.gateway-server-chat-b.test.ts +++ b/src/gateway/server.chat.gateway-server-chat-b.test.ts @@ -2416,6 +2416,224 @@ describe("gateway server chat", () => { } }); + test("chat.send terminalizes the client run when a followup is queued", async () => { + const sessionDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-gw-")); + try { + testState.sessionStorePath = path.join(sessionDir, "sessions.json"); + await writeSessionStore({ + entries: { + main: { + sessionId: "sess-main", + updatedAt: Date.now(), + }, + }, + }); + + const broadcast = vi.fn((_event: string, _payload: unknown) => undefined); + const context = { + loadGatewayModelCatalog: vi.fn(), + logGateway: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + debug: vi.fn(), + }, + agentRunSeq: new Map(), + chatAbortControllers: new Map(), + chatAbortedRuns: new Map(), + chatQueuedTurns: new Map(), + chatRunBuffers: new Map(), + chatDeltaSentAt: new Map(), + chatDeltaLastBroadcastLen: new Map(), + chatDeltaLastBroadcastText: new Map(), + agentDeltaSentAt: new Map(), + bufferedAgentEvents: new Map(), + clearChatRunState: vi.fn(), + addChatRun: vi.fn(), + removeChatRun: vi.fn(), + broadcast, + broadcastToConnIds: vi.fn(), + nodeSendToSession: vi.fn(), + registerToolEventRecipient: vi.fn(), + getRuntimeConfig: () => ({}), + dedupe: new Map(), + } as unknown as GatewayRequestContext; + let queuedLifecycle: GetReplyOptions["queuedFollowupLifecycle"]; + const dispatchRelease = createDeferred(); + dispatchInboundMessageMock.mockImplementationOnce(async (args: unknown) => { + queuedLifecycle = (args as { replyOptions?: GetReplyOptions }).replyOptions + ?.queuedFollowupLifecycle; + queuedLifecycle?.onEnqueued?.(); + await dispatchRelease.promise; + return {}; + }); + + const { chatHandlers } = await import("./server-methods/chat.js"); + await chatHandlers["chat.send"]({ + req: { + type: "req", + id: "queued-followup", + method: "chat.send", + params: { + sessionKey: "main", + message: "queued prompt", + idempotencyKey: "idem-queued-followup", + }, + }, + params: { + sessionKey: "main", + message: "queued prompt", + idempotencyKey: "idem-queued-followup", + }, + client: { + connId: "conn-tui", + connect: { + client: { + id: GATEWAY_CLIENT_NAMES.TUI, + mode: GATEWAY_CLIENT_MODES.UI, + }, + scopes: ["operator.write", "operator.admin"], + }, + } as never, + isWebchatConnect: () => true, + respond: vi.fn() as RespondFn, + context, + }); + + await vi.waitFor(() => expect(queuedLifecycle).toBeDefined(), FAST_WAIT_OPTS); + expect(queuedLifecycle?.ownerKey).toBe("connection:conn-tui"); + expect(broadcast).not.toHaveBeenCalledWith( + "chat", + expect.objectContaining({ runId: "idem-queued-followup", state: "final" }), + ); + dispatchRelease.resolve(); + await vi.waitFor(() => { + expect(broadcast).toHaveBeenCalledWith( + "chat", + expect.objectContaining({ + runId: "idem-queued-followup", + sessionKey: "agent:main:main", + state: "final", + }), + ); + }, FAST_WAIT_OPTS); + const finalEvents = broadcast.mock.calls.filter( + ([event, payload]) => + event === "chat" && + (payload as { runId?: string; state?: string }).runId === "idem-queued-followup" && + (payload as { state?: string }).state === "final", + ); + expect(finalEvents).toHaveLength(1); + expect(context.chatQueuedTurns.has("idem-queued-followup")).toBe(true); + + context.dedupe.delete("chat:idem-queued-followup"); + const replayRespond = vi.fn() as RespondFn; + await chatHandlers["chat.send"]({ + req: { + type: "req", + id: "queued-followup-replay", + method: "chat.send", + params: { + sessionKey: "main", + message: "queued prompt", + idempotencyKey: "idem-queued-followup", + }, + }, + params: { + sessionKey: "main", + message: "queued prompt", + idempotencyKey: "idem-queued-followup", + }, + client: { + connId: "conn-tui", + connect: { + client: { id: GATEWAY_CLIENT_NAMES.TUI, mode: GATEWAY_CLIENT_MODES.UI }, + scopes: ["operator.write", "operator.admin"], + }, + } as never, + isWebchatConnect: () => true, + respond: replayRespond, + context, + }); + expect(replayRespond).toHaveBeenCalledWith( + true, + { runId: "idem-queued-followup", status: "in_flight" }, + undefined, + { cached: true, runId: "idem-queued-followup" }, + ); + expect(dispatchInboundMessageMock).toHaveBeenCalledTimes(1); + + queuedLifecycle?.onComplete?.(); + expect(context.chatQueuedTurns.has("idem-queued-followup")).toBe(false); + await vi.waitFor( + () => expect(context.removeChatRun).toHaveBeenCalledTimes(1), + FAST_WAIT_OPTS, + ); + + let failedDispatchLifecycle: GetReplyOptions["queuedFollowupLifecycle"]; + dispatchInboundMessageMock.mockImplementationOnce(async (args: unknown) => { + failedDispatchLifecycle = (args as { replyOptions?: GetReplyOptions }).replyOptions + ?.queuedFollowupLifecycle; + failedDispatchLifecycle?.onEnqueued?.(); + throw new Error("post-enqueue bookkeeping failed"); + }); + await chatHandlers["chat.send"]({ + req: { + type: "req", + id: "queued-followup-post-error", + method: "chat.send", + params: { + sessionKey: "main", + message: "accepted before dispatch error", + idempotencyKey: "idem-queued-followup-post-error", + }, + }, + params: { + sessionKey: "main", + message: "accepted before dispatch error", + idempotencyKey: "idem-queued-followup-post-error", + }, + client: { + connId: "conn-tui", + connect: { + client: { + id: GATEWAY_CLIENT_NAMES.TUI, + mode: GATEWAY_CLIENT_MODES.UI, + }, + scopes: ["operator.write", "operator.admin"], + }, + } as never, + isWebchatConnect: () => true, + respond: vi.fn() as RespondFn, + context, + }); + + await vi.waitFor( + () => expect(context.removeChatRun).toHaveBeenCalledTimes(2), + FAST_WAIT_OPTS, + ); + const acceptedErrorEvents = broadcast.mock.calls.filter( + ([event, payload]) => + event === "chat" && + (payload as { runId?: string }).runId === "idem-queued-followup-post-error", + ); + expect(acceptedErrorEvents).toHaveLength(1); + expect(acceptedErrorEvents[0]?.[1]).toMatchObject({ state: "final" }); + expect(context.dedupe.get("chat:idem-queued-followup-post-error")).toMatchObject({ + ok: true, + payload: { status: "ok" }, + }); + expect(context.chatQueuedTurns.has("idem-queued-followup-post-error")).toBe(true); + failedDispatchLifecycle?.onComplete?.(); + expect(context.chatQueuedTurns.has("idem-queued-followup-post-error")).toBe(false); + } finally { + dispatchInboundMessageMock.mockReset(); + testState.sessionStorePath = undefined; + clearConfigCache(); + await removeTempDir(sessionDir); + } + }); + test("chat.send emits operator-only post-ACK server timing milestones", async () => { const sessionDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-gw-")); try { diff --git a/src/gateway/server.impl.ts b/src/gateway/server.impl.ts index 837271f601d..6c0e717ec10 100644 --- a/src/gateway/server.impl.ts +++ b/src/gateway/server.impl.ts @@ -896,6 +896,7 @@ export async function startGatewayServer( addChatRun, removeChatRun, chatAbortControllers, + chatQueuedTurns, toolEventRecipients, } = await startupTrace.measure("runtime.state", () => createGatewayRuntimeState({ @@ -1065,6 +1066,7 @@ export async function startGatewayServer( lifecycleUnsub: runtimeState.lifecycleUnsub, chatRunState, chatAbortControllers, + chatQueuedTurns, restartRecoveryCandidates, removeChatRun, agentRunSeq, @@ -1137,6 +1139,7 @@ export async function startGatewayServer( logHealth, dedupe, chatAbortControllers, + chatQueuedTurns, restartRecoveryCandidates, chatRunState, chatRunBuffers, @@ -1466,6 +1469,7 @@ export async function startGatewayServer( terminalSessions, agentRunSeq, chatAbortControllers, + chatQueuedTurns, chatAbortedRuns: chatRunState.abortedRuns, chatRunBuffers: chatRunState.buffers, chatDeltaSentAt: chatRunState.deltaSentAt, diff --git a/src/gateway/test-helpers.maintenance-state.ts b/src/gateway/test-helpers.maintenance-state.ts index 9d8ff467053..01e91491f78 100644 --- a/src/gateway/test-helpers.maintenance-state.ts +++ b/src/gateway/test-helpers.maintenance-state.ts @@ -20,6 +20,7 @@ export function createGatewayMaintenanceStateForTest(params?: { logHealth: { error: () => {} }, dedupe: new Map(), chatAbortControllers: new Map(), + chatQueuedTurns: new Map(), restartRecoveryCandidates: new Map(), chatRunState, chatRunBuffers: chatRunState.buffers, diff --git a/src/sessions/user-turn-transcript.ts b/src/sessions/user-turn-transcript.ts index 22b0533b56f..7287f265c22 100644 --- a/src/sessions/user-turn-transcript.ts +++ b/src/sessions/user-turn-transcript.ts @@ -170,19 +170,20 @@ function resolveTranscriptMediaType(params: { } export function buildPersistedUserTurnMediaInputsFromFields( - fields: PersistedUserTurnMediaFieldSource | null | undefined, + fields: PersistedUserTurnMediaFieldSource | PersistedUserTurnMessage | null | undefined, ): PersistedUserTurnMediaInput[] { if (!fields) { return []; } - const paths = normalizeOptionalTextArray(fields.MediaPaths); - const urls = normalizeOptionalTextArray(fields.MediaUrls); - const types = normalizeOptionalTextArray(fields.MediaTypes); - const singlePath = normalizeOptionalText(fields.MediaPath); - const singleUrl = normalizeOptionalText(fields.MediaUrl); - const singleType = normalizeOptionalText(fields.MediaType); - const workspaceDir = normalizeOptionalText(fields.MediaWorkspaceDir); + const mediaFields = fields as PersistedUserTurnMediaFieldSource; + const paths = normalizeOptionalTextArray(mediaFields.MediaPaths); + const urls = normalizeOptionalTextArray(mediaFields.MediaUrls); + const types = normalizeOptionalTextArray(mediaFields.MediaTypes); + const singlePath = normalizeOptionalText(mediaFields.MediaPath); + const singleUrl = normalizeOptionalText(mediaFields.MediaUrl); + const singleType = normalizeOptionalText(mediaFields.MediaType); + const workspaceDir = normalizeOptionalText(mediaFields.MediaWorkspaceDir); const mediaCount = Math.max(paths.length, urls.length, singlePath || singleUrl ? 1 : 0); const media: PersistedUserTurnMediaInput[] = []; diff --git a/src/tui/embedded-backend.test.ts b/src/tui/embedded-backend.test.ts index 36ef13ee76d..28c769db118 100644 --- a/src/tui/embedded-backend.test.ts +++ b/src/tui/embedded-backend.test.ts @@ -1378,13 +1378,13 @@ describe("EmbeddedTuiBackend", () => { agentId: "work", runId: "run-local-default-global", }), - ).resolves.toEqual({ ok: true, aborted: false }); + ).resolves.toEqual({ ok: true, aborted: false, runIds: [] }); await expect( backend.abortChat({ sessionKey: "global", runId: "run-local-work-global", }), - ).resolves.toEqual({ ok: true, aborted: false }); + ).resolves.toEqual({ ok: true, aborted: false, runIds: [] }); expect(defaultAbortListener).not.toHaveBeenCalled(); expect(workAbortListener).not.toHaveBeenCalled(); @@ -1994,10 +1994,66 @@ describe("EmbeddedTuiBackend", () => { }); await flushMicrotasks(); - expect(result).toEqual({ ok: true, aborted: true }); + expect(result).toEqual({ ok: true, aborted: true, runIds: ["run-abort-1"] }); expect(capturedSignal?.aborted).toBe(true); }); + it("keeps local BTW runs alive during a session-scoped abort", async () => { + const { EmbeddedTuiBackend } = await import("./embedded-backend.js"); + loadSessionEntryMock.mockReturnValue({ + cfg: {}, + canonicalKey: "agent:main:main", + storePath: "/tmp/openclaw-sessions.json", + store: {}, + entry: { sessionId: "session-main" }, + }); + const mainRun = deferred<{ payloads: Array<{ text: string }>; meta: { aborted?: boolean } }>(); + const btwRun = deferred<{ text: string }>(); + let mainSignal: AbortSignal | undefined; + let btwSignal: AbortSignal | undefined; + agentCommandFromIngressMock.mockImplementationOnce((opts: { abortSignal?: AbortSignal }) => { + mainSignal = opts.abortSignal; + opts.abortSignal?.addEventListener( + "abort", + () => mainRun.resolve({ payloads: [], meta: { aborted: true } }), + { once: true }, + ); + return mainRun.promise; + }); + runBtwSideQuestionMock.mockImplementationOnce( + (params: { opts?: { abortSignal?: AbortSignal } }) => { + btwSignal = params.opts?.abortSignal; + return btwRun.promise; + }, + ); + + const backend = new EmbeddedTuiBackend(); + backend.start(); + await backend.sendChat({ + sessionKey: "agent:main:main", + message: "long task", + runId: "run-main-abort", + }); + await backend.sendChat({ + sessionKey: "agent:main:main", + message: "/btw what changed?", + runId: "run-btw-survives", + }); + await vi.waitFor(() => { + expect(agentCommandFromIngressMock).toHaveBeenCalledTimes(1); + expect(runBtwSideQuestionMock).toHaveBeenCalledTimes(1); + }); + + const result = await backend.abortChat({ sessionKey: "agent:main:main" }); + + expect(result).toEqual({ ok: true, aborted: true, runIds: ["run-main-abort"] }); + expect(mainSignal?.aborted).toBe(true); + expect(btwSignal?.aborted).toBe(false); + + btwRun.resolve({ text: "still running" }); + await flushMicrotasks(); + }); + it("passes explicit chat timeouts to the agent command as seconds", async () => { const { EmbeddedTuiBackend } = await import("./embedded-backend.js"); agentCommandFromIngressMock.mockResolvedValueOnce({ diff --git a/src/tui/embedded-backend.ts b/src/tui/embedded-backend.ts index 9e3e6e39d8f..3459d27db33 100644 --- a/src/tui/embedded-backend.ts +++ b/src/tui/embedded-backend.ts @@ -426,24 +426,52 @@ export class EmbeddedTuiBackend implements TuiBackend { return { runId }; } - async abortChat(opts: { sessionKey: string; agentId?: string; runId: string }) { + async abortChat(opts: { sessionKey: string; agentId?: string; runId?: string }) { + if (!opts.runId) { + // Session-scoped abort for local embedded: abort all matching runs. + let aborted = false; + const runIds: string[] = []; + for (const [runId, run] of this.runs) { + if (run.isBtw) { + continue; + } + if (run.sessionKey !== opts.sessionKey) { + continue; + } + if (opts.sessionKey === "global") { + const defaultAgentId = resolveDefaultAgentId(getRuntimeConfig()); + const requestedAgentId = opts.agentId ? normalizeAgentId(opts.agentId) : defaultAgentId; + const runAgentId = run.agentId ? normalizeAgentId(run.agentId) : defaultAgentId; + if (runAgentId !== requestedAgentId) { + continue; + } + } + if (!this.isAbortableRun(runId, run)) { + continue; + } + run.controller.abort(); + aborted = true; + runIds.push(runId); + } + return { ok: true, aborted, runIds }; + } const run = this.runs.get(opts.runId); if (!run || run.sessionKey !== opts.sessionKey) { - return { ok: true, aborted: false }; + return { ok: true, aborted: false, runIds: [] }; } if (opts.sessionKey === "global") { const defaultAgentId = resolveDefaultAgentId(getRuntimeConfig()); const requestedAgentId = opts.agentId ? normalizeAgentId(opts.agentId) : defaultAgentId; const runAgentId = run.agentId ? normalizeAgentId(run.agentId) : defaultAgentId; if (runAgentId !== requestedAgentId) { - return { ok: true, aborted: false }; + return { ok: true, aborted: false, runIds: [] }; } } if (!this.isAbortableRun(opts.runId, run)) { - return { ok: true, aborted: false }; + return { ok: true, aborted: false, runIds: [] }; } run.controller.abort(); - return { ok: true, aborted: true }; + return { ok: true, aborted: true, runIds: [opts.runId] }; } async loadHistory(opts: { sessionKey: string; agentId?: string; limit?: number }) { diff --git a/src/tui/gateway-chat.test.ts b/src/tui/gateway-chat.test.ts index 2c16ee968a2..55d0d4fe4b4 100644 --- a/src/tui/gateway-chat.test.ts +++ b/src/tui/gateway-chat.test.ts @@ -655,6 +655,52 @@ describe("GatewayChatClient", () => { }); }); + it("preserves side runs for session-scoped TUI aborts", async () => { + const client = new GatewayChatClient({ + url: "ws://127.0.0.1:18789", + token: "test-token", + allowInsecureLocalOperatorUi: true, + }); + const request = vi.fn().mockResolvedValue({ ok: true, aborted: true }); + (client as unknown as { client: { request: typeof request } }).client.request = request; + + await client.abortChat({ sessionKey: "main" }); + + expect(request).toHaveBeenCalledWith("chat.abort", { + sessionKey: "main", + preserveSideRuns: true, + }); + }); + + it("retries session aborts without side-run preservation on older Gateways", async () => { + const client = new GatewayChatClient({ + url: "ws://127.0.0.1:18789", + token: "test-token", + allowInsecureLocalOperatorUi: true, + }); + const request = vi + .fn() + .mockRejectedValueOnce( + new GatewayClientRequestError({ + code: "INVALID_REQUEST", + message: "invalid chat.abort params: at root: unexpected property 'preserveSideRuns'", + }), + ) + .mockResolvedValueOnce({ ok: true, aborted: true, runIds: ["run-main"] }); + (client as unknown as { client: { request: typeof request } }).client.request = request; + + await expect(client.abortChat({ sessionKey: "main" })).resolves.toEqual({ + ok: true, + aborted: true, + runIds: ["run-main"], + }); + expect(request).toHaveBeenNthCalledWith(1, "chat.abort", { + sessionKey: "main", + preserveSideRuns: true, + }); + expect(request).toHaveBeenNthCalledWith(2, "chat.abort", { sessionKey: "main" }); + }); + it("returns the actual chat send ack status from the gateway", async () => { const client = new GatewayChatClient({ url: "ws://127.0.0.1:18789", diff --git a/src/tui/gateway-chat.ts b/src/tui/gateway-chat.ts index 19c6a467467..05dce41844d 100644 --- a/src/tui/gateway-chat.ts +++ b/src/tui/gateway-chat.ts @@ -100,6 +100,14 @@ function nonEmptyString(value: unknown): string | undefined { return typeof value === "string" && value.trim().length > 0 ? value.trim() : undefined; } +function isLegacyPreserveSideRunsError(err: unknown): boolean { + if (!(err instanceof GatewayClientRequestError) || err.gatewayCode !== "INVALID_REQUEST") { + return false; + } + const message = err.message.toLowerCase(); + return message.includes("invalid chat.abort params") && message.includes("preservesideruns"); +} + export type GatewaySessionList = TuiSessionList; export type GatewayAgentsList = TuiAgentsList; export type GatewayModelChoice = TuiModelChoice; @@ -211,12 +219,34 @@ export class GatewayChatClient implements TuiBackend { return status ? { runId: acceptedRunId, status } : { runId: acceptedRunId }; } - async abortChat(opts: { sessionKey: string; agentId?: string; runId: string }) { - return await this.client.request<{ ok: boolean; aborted: boolean }>("chat.abort", { + async abortChat(opts: { sessionKey: string; agentId?: string; runId?: string }) { + const params = { sessionKey: opts.sessionKey, ...(opts.agentId ? { agentId: opts.agentId } : {}), - runId: opts.runId, - }); + ...(opts.runId ? { runId: opts.runId } : {}), + }; + if (opts.runId) { + return await this.client.request<{ ok: boolean; aborted: boolean; runIds?: string[] }>( + "chat.abort", + params, + ); + } + try { + return await this.client.request<{ ok: boolean; aborted: boolean; runIds?: string[] }>( + "chat.abort", + { ...params, preserveSideRuns: true }, + ); + } catch (err) { + // Protocol v4 peers reject unknown fields. Retry the shipped abort shape + // so mixed-version TUI stops still work, even without BTW isolation. + if (!isLegacyPreserveSideRunsError(err)) { + throw err; + } + return await this.client.request<{ ok: boolean; aborted: boolean; runIds?: string[] }>( + "chat.abort", + params, + ); + } } async loadHistory(opts: { sessionKey: string; agentId?: string; limit?: number }) { diff --git a/src/tui/tui-backend.ts b/src/tui/tui-backend.ts index 4b427c0eca2..2b55719d764 100644 --- a/src/tui/tui-backend.ts +++ b/src/tui/tui-backend.ts @@ -1,3 +1,4 @@ +import type { FastMode } from "@openclaw/normalization-core/string-coerce"; // Defines the TUI backend contract and backend event shapes. import type { CommandEntry, @@ -6,7 +7,6 @@ import type { SessionsPatchParams, SessionsPatchResult, } from "../../packages/gateway-protocol/src/index.js"; -import type { FastMode } from "@openclaw/normalization-core/string-coerce"; import type { ResponseUsageMode, SessionInfo, SessionScope } from "./tui-types.js"; // Transport-agnostic backend contract consumed by the TUI runtime. @@ -149,11 +149,12 @@ export type TuiBackend = { stop: () => void | Promise; subscribeSessionEvents?: () => Promise; sendChat: (opts: ChatSendOptions) => Promise; + /** runId optional: omit for session-scoped abort (queued turns then active). */ abortChat: (opts: { sessionKey: string; agentId?: string; - runId: string; - }) => Promise<{ ok: boolean; aborted: boolean }>; + runId?: string; + }) => Promise<{ ok: boolean; aborted: boolean; runIds?: string[] }>; loadHistory: (opts: { sessionKey: string; agentId?: string; limit?: number }) => Promise; listSessions: (opts?: SessionsListParams) => Promise; listAgents: () => Promise; diff --git a/src/tui/tui-command-handlers.test.ts b/src/tui/tui-command-handlers.test.ts index 7b51245105d..5e3696a1df2 100644 --- a/src/tui/tui-command-handlers.test.ts +++ b/src/tui/tui-command-handlers.test.ts @@ -1110,7 +1110,7 @@ describe("tui command handlers", () => { expect(state.pendingChatRunId).toEqual(expect.any(String)); }); - it("blocks gateway slash prompts while a run is active", async () => { + it("forwards gateway slash prompts while a run is active", async () => { const { handleCommand, sendChat, addPendingUser, addSystem } = createHarness({ activeChatRunId: "run-active", activityStatus: "streaming", @@ -1118,11 +1118,10 @@ describe("tui command handlers", () => { await handleCommand("/context detail"); - expect(sendChat).not.toHaveBeenCalled(); - expect(addPendingUser).not.toHaveBeenCalled(); - expect(addSystem).toHaveBeenCalledWith( + expectSendChatFields(sendChat, { message: "/context detail" }); + expect(addPendingUser).toHaveBeenCalledWith(expect.any(String), "/context detail"); + expect(addSystem).not.toHaveBeenCalledWith( "agent is busy — press Esc to abort before sending a new message", - { coalesceConsecutive: true }, ); }); @@ -1141,19 +1140,15 @@ describe("tui command handlers", () => { expect(addUser).not.toHaveBeenCalled(); }); - it("sends slash stop to the backend when there is no tracked run", async () => { + it("routes slash stop to session abort when there is no tracked run", async () => { const abortActive = vi.fn().mockResolvedValue(undefined); const { handleCommand, sendChat, addPendingUser } = createHarness({ abortActive }); await handleCommand("/stop"); - expect(abortActive).not.toHaveBeenCalled(); - expect(sendChat).toHaveBeenCalledTimes(1); - expectSendChatFields(sendChat, { - message: "/stop", - sessionKey: "agent:main:main", - }); - expect(addPendingUser).toHaveBeenCalledWith(expect.any(String), "/stop"); + expect(abortActive).toHaveBeenCalledWith({ preferActive: true }); + expect(sendChat).not.toHaveBeenCalled(); + expect(addPendingUser).not.toHaveBeenCalled(); }); it("sends broad stop-like text as a normal prompt when idle", async () => { @@ -1200,22 +1195,102 @@ describe("tui command handlers", () => { ); }); - it("blocks gateway sends while the current run is finishing", async () => { - const { handleCommand, sendChat, addUser, addSystem } = createHarness({ + it("forwards gateway sends while the current run is finishing", async () => { + const { handleCommand, sendChat, addPendingUser, addSystem } = createHarness({ activeChatRunId: "run-active", activityStatus: "finishing context", }); await handleCommand("/context detail"); + expect(sendChat).toHaveBeenCalledTimes(1); + expect(addPendingUser).toHaveBeenCalledWith(expect.any(String), "/context detail"); + expect(addSystem).not.toHaveBeenCalledWith( + "agent is busy — press Esc to abort before sending a new message", + ); + }); + + it("forwards gateway sends while a run is active so Gateway owns queue policy", async () => { + const { handleCommand, sendChat, addPendingUser, addSystem } = createHarness({ + activeChatRunId: "run-active", + activityStatus: "streaming", + }); + + await handleCommand("follow up question"); + + expect(sendChat).toHaveBeenCalledTimes(1); + expect(addPendingUser).toHaveBeenCalledWith(expect.any(String), "follow up question"); + expect(addSystem).not.toHaveBeenCalledWith( + "agent is busy — press Esc to abort before sending a new message", + ); + }); + + it("blocks sends while optimistic user message admission is pending", async () => { + const { handleCommand, sendChat, addSystem } = createHarness({ + activeChatRunId: "run-active", + pendingOptimisticUserMessage: true, + activityStatus: "sending", + }); + + await handleCommand("another message"); + expect(sendChat).not.toHaveBeenCalled(); - expect(addUser).not.toHaveBeenCalled(); expect(addSystem).toHaveBeenCalledWith( "agent is busy — press Esc to abort before sending a new message", { coalesceConsecutive: true }, ); }); + it("preserves activeChatRunId when a queued followup send fails", async () => { + const sendChat = vi.fn().mockRejectedValue(new Error("network error")); + const { handleCommand, addSystem, state } = createHarness({ + sendChat, + activeChatRunId: "run-active", + activityStatus: "streaming", + }); + + await handleCommand("queued followup"); + + expect(addSystem).toHaveBeenCalledWith(expect.stringContaining("send failed")); + expect(state.activeChatRunId).toBe("run-active"); + }); + + it("does not restore a queued run that completes before the followup send fails", async () => { + let rejectSend: (error: Error) => void = () => { + throw new Error("sendChat promise rejector was not initialized"); + }; + const sendChat = vi.fn( + () => + new Promise((_resolve, reject) => { + rejectSend = reject; + }), + ); + const { handleCommand, state } = createHarness({ + sendChat, + activeChatRunId: "run-active", + activityStatus: "streaming", + }); + + const pendingSend = handleCommand("queued followup"); + await Promise.resolve(); + state.activeChatRunId = null; + rejectSend(new Error("network error")); + await pendingSend; + + expect(state.activeChatRunId).toBeNull(); + }); + + it("clears activeChatRunId when a non-queued send fails", async () => { + const sendChat = vi.fn().mockRejectedValue(new Error("network error")); + const { handleCommand, state } = createHarness({ + sendChat, + }); + + await handleCommand("some message"); + + expect(state.activeChatRunId).toBeNull(); + }); + it("runs /auth through the local auth flow and refreshes session info", async () => { const refreshSessionInfo = vi.fn().mockResolvedValue(undefined); const runAuthFlow = vi.fn().mockResolvedValue({ exitCode: 0, signal: null }); @@ -1496,4 +1571,62 @@ describe("tui command handlers", () => { expect(patchSession).toHaveBeenCalledWith(expect.objectContaining({ responseUsage: "full" })); expect(addSystem).toHaveBeenCalledWith("usage footer: full"); }); + + it("allows /queue directives to reach gateway during an active run in steer mode", async () => { + const { handleCommand, sendChat, addPendingUser, addSystem } = createHarness({ + activeChatRunId: "run-active", + activityStatus: "streaming", + }); + + await handleCommand("/queue followup"); + + expect(sendChat).toHaveBeenCalledTimes(1); + expectSendChatFields(sendChat, { message: "/queue followup" }); + expect(addPendingUser).toHaveBeenCalledWith(expect.any(String), "/queue followup"); + expect(addSystem).not.toHaveBeenCalledWith( + "agent is busy — press Esc to abort before sending a new message", + ); + }); + + it("allows bare /queue to reach gateway during an active run", async () => { + const { handleCommand, sendChat, addSystem } = createHarness({ + activeChatRunId: "run-active", + activityStatus: "streaming", + }); + + await handleCommand("/queue"); + + expect(sendChat).toHaveBeenCalledTimes(1); + expectSendChatFields(sendChat, { message: "/queue" }); + expect(addSystem).not.toHaveBeenCalledWith( + "agent is busy — press Esc to abort before sending a new message", + ); + }); + + it("allows colon-form /queue directives during an active run", async () => { + const { handleCommand, sendChat } = createHarness({ + activeChatRunId: "run-active", + activityStatus: "streaming", + }); + + await handleCommand("/queue:followup"); + + expectSendChatFields(sendChat, { message: "/queue:followup" }); + }); + + it("blocks /queue while optimistic user message is pending", async () => { + const { handleCommand, sendChat, addSystem } = createHarness({ + activeChatRunId: "run-active", + pendingOptimisticUserMessage: true, + activityStatus: "sending", + }); + + await handleCommand("/queue followup"); + + expect(sendChat).not.toHaveBeenCalled(); + expect(addSystem).toHaveBeenCalledWith( + "agent is busy — press Esc to abort before sending a new message", + { coalesceConsecutive: true }, + ); + }); }); diff --git a/src/tui/tui-command-handlers.ts b/src/tui/tui-command-handlers.ts index 6c56b0e9be5..6d515efd94b 100644 --- a/src/tui/tui-command-handlers.ts +++ b/src/tui/tui-command-handlers.ts @@ -735,11 +735,9 @@ export function createCommandHandlers(context: CommandHandlerContext) { await abortActive(); break; case "stop": - if (hasTrackedAbortTarget()) { - await abortActive({ preferActive: true }); - break; - } - await sendMessage(raw); + // Queued client runs can terminalize before the followup executes, so + // local run ids are not a complete stop target inventory. + await abortActive({ preferActive: true }); break; case "settings": openSettings(); @@ -771,18 +769,15 @@ export function createCommandHandlers(context: CommandHandlerContext) { state.activeChatRunId || state.pendingChatRunId || state.pendingOptimisticUserMessage, ); if ( - hasTrackedAbortTarget() && - (isSlashStopCommand(text) || (busy && isChatStopCommandText(text))) + isSlashStopCommand(text) || + (hasTrackedAbortTarget() && busy && isChatStopCommandText(text)) ) { await abortActive({ preferActive: true }); return; } - if ( - !isBtw && - (state.pendingChatRunId || - state.pendingOptimisticUserMessage || - (opts.local !== true && state.activeChatRunId)) - ) { + // The Gateway owns queue policy. TUI only serializes pending RPC admission; + // an already-active run must not suppress steer/followup/collect/interrupt. + if (!isBtw && (state.pendingOptimisticUserMessage || state.pendingChatRunId)) { addBlockedChatSubmitNotice(chatLog); tui.requestRender(); return; @@ -900,7 +895,7 @@ export function createCommandHandlers(context: CommandHandlerContext) { if (isBtw) { forgetLocalBtwRunId?.(runId); } - if (!isBtw && state.activeChatRunId) { + if (!isBtw && state.activeChatRunId && state.activeChatRunId === runId) { forgetLocalRunId?.(state.activeChatRunId); } if (!isBtw) { @@ -909,7 +904,11 @@ export function createCommandHandlers(context: CommandHandlerContext) { if (!isBtw) { state.pendingOptimisticUserMessage = false; state.pendingChatRunId = null; - state.activeChatRunId = null; + // Only clear the failed send's ownership. A queued run may have + // terminalized or handed ownership off while the RPC was pending. + if (state.activeChatRunId === runId) { + state.activeChatRunId = null; + } if (state.pendingSubmitDraft?.runId === runId) { state.pendingSubmitDraft = null; } diff --git a/src/tui/tui-pty-local.e2e.test.ts b/src/tui/tui-pty-local.e2e.test.ts index f1adba89ac3..4101875ce2b 100644 --- a/src/tui/tui-pty-local.e2e.test.ts +++ b/src/tui/tui-pty-local.e2e.test.ts @@ -1,12 +1,14 @@ -// Exercises the slower TUI local-mode PTY smoke path. +// Exercises slower TUI PTY paths against real local and Gateway backends. import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import { createServer, type IncomingMessage, type ServerResponse } from "node:http"; import { tmpdir } from "node:os"; import path from "node:path"; import { pathToFileURL } from "node:url"; import { afterEach, describe, expect, it } from "vitest"; +import { createOpenClawTestInstance } from "../../test/helpers/openclaw-test-instance.js"; import type { OpenClawConfig } from "../config/types.openclaw.js"; -import { startPty, waitFor, type PtyRun } from "./tui-pty-test-support.js"; +import { GatewayChatClient } from "./gateway-chat.js"; +import { sleep, startPty, waitFor, type PtyRun } from "./tui-pty-test-support.js"; type MockModelServer = { baseUrl: string; @@ -44,7 +46,7 @@ function writeJson(res: ServerResponse, status: number, body: unknown) { res.end(text); } -function writeResponsesSse(res: ServerResponse, text: string) { +async function writeResponsesSse(res: ServerResponse, text: string, completionDelayMs = 0) { const id = "msg_tui_pty_local"; const events = [ { @@ -93,14 +95,23 @@ function writeResponsesSse(res: ServerResponse, text: string) { }, }, ]; - const body = `${events.map((event) => `data: ${JSON.stringify(event)}\n\n`).join("")}data: [DONE]\n\n`; res.writeHead(200, { "content-type": "text/event-stream", "cache-control": "no-store", connection: "keep-alive", - "content-length": Buffer.byteLength(body), }); - res.end(body); + res.write(`data: ${JSON.stringify(events[0])}\n\n`); + if (completionDelayMs > 0) { + await sleep(completionDelayMs); + } + if (res.destroyed) { + return; + } + const completionBody = `${events + .slice(1) + .map((event) => `data: ${JSON.stringify(event)}\n\n`) + .join("")}data: [DONE]\n\n`; + res.end(completionBody); } async function readJsonRequest(req: IncomingMessage): Promise> { @@ -108,7 +119,10 @@ async function readJsonRequest(req: IncomingMessage): Promise) : {}; } -async function startMockModelServer(replyText: string): Promise { +async function startMockModelServer( + replyText: string, + opts: { firstResponseDelayMs?: number; followupReplyText?: string } = {}, +): Promise { const requests: MockModelRequest[] = []; const server = createServer((req, res) => { void (async () => { @@ -123,9 +137,14 @@ async function startMockModelServer(replyText: string): Promise } if (req.method === "POST") { const body = await readJsonRequest(req); + const requestIndex = requests.length; requests.push({ method: req.method, path: url.pathname, body }); if (url.pathname === "/v1/responses" || url.pathname === "/responses") { - writeResponsesSse(res, replyText); + await writeResponsesSse( + res, + requestIndex === 0 ? replyText : (opts.followupReplyText ?? replyText), + requestIndex === 0 ? (opts.firstResponseDelayMs ?? 0) : 0, + ); return; } writeJson(res, 404, { error: "not found" }); @@ -154,6 +173,21 @@ async function startMockModelServer(replyText: string): Promise }; } +function buildTuiCliScript(args: string[]) { + const tuiCliModuleUrl = pathToFileURL(path.join(process.cwd(), "src/cli/tui-cli.ts")).href; + return [ + `import { Command } from "commander";`, + `import { registerTuiCli } from ${JSON.stringify(tuiCliModuleUrl)};`, + `const program = new Command();`, + `program.exitOverride();`, + `registerTuiCli(program);`, + `program.parseAsync([process.execPath, "openclaw", ...${JSON.stringify(args)}], { from: "node" }).catch((error) => {`, + ` console.error(error);`, + ` process.exit(1);`, + `});`, + ].join("\n"); +} + function buildLocalModeConfig(params: { workspaceDir: string; providerBaseUrl: string }) { return { plugins: { @@ -227,18 +261,7 @@ async function startLocalModeTui() { const configPath = path.join(tempDir, "openclaw.json"); const mockModel = await startMockModelServer(replyText); const config = buildLocalModeConfig({ workspaceDir, providerBaseUrl: mockModel.baseUrl }); - const tuiCliModuleUrl = pathToFileURL(path.join(process.cwd(), "src/cli/tui-cli.ts")).href; - const script = [ - `import { Command } from "commander";`, - `import { registerTuiCli } from ${JSON.stringify(tuiCliModuleUrl)};`, - `const program = new Command();`, - `program.exitOverride();`, - `registerTuiCli(program);`, - `program.parseAsync([process.execPath, "openclaw", "tui", "--local"], { from: "node" }).catch((error) => {`, - ` console.error(error);`, - ` process.exit(1);`, - `});`, - ].join("\n"); + const script = buildTuiCliScript(["tui", "--local"]); await Promise.all([ mkdir(workspaceDir, { recursive: true }), mkdir(homeDir, { recursive: true }), @@ -279,7 +302,78 @@ async function startLocalModeTui() { }; } -describe("TUI PTY local mode", () => { +async function startGatewayModeTui(params: { + queueMode: "followup" | "collect"; + firstResponseDelayMs?: number; + queueDebounceMs?: number; +}) { + const tempDir = await mkdtemp(path.join(tmpdir(), "openclaw-tui-pty-gateway-")); + const workspaceDir = path.join(tempDir, "workspace"); + const mockModel = await startMockModelServer("FIRST_RUN_ACTIVE", { + firstResponseDelayMs: params.firstResponseDelayMs ?? 2_500, + followupReplyText: "FOLLOWUP_RUN_COMPLETE", + }); + const config = { + ...buildLocalModeConfig({ workspaceDir, providerBaseUrl: mockModel.baseUrl }), + messages: { + queue: { + mode: params.queueMode, + debounceMs: params.queueDebounceMs ?? 25, + }, + }, + } satisfies OpenClawConfig; + const gateway = await createOpenClawTestInstance({ + name: `tui-pty-gateway-${params.queueMode}`, + gatewayToken: "tui-pty-local", + config, + env: { + OPENCLAW_CODEX_DISCOVERY_LIVE: "0", + OPENCLAW_SKIP_PROVIDERS: undefined, + }, + }); + try { + await mkdir(workspaceDir, { recursive: true }); + await gateway.startGateway(); + const script = buildTuiCliScript([ + "tui", + "--url", + gateway.url, + "--token", + gateway.gatewayToken, + "--session", + "agent:main:main", + ]); + const run = startPty(process.execPath, ["--import", "tsx", "--eval", script], { + activeRuns, + cwd: process.cwd(), + env: { + ...gateway.env, + OPENCLAW_THEME: "dark", + NO_COLOR: undefined, + }, + exitTimeoutMs: LOCAL_EXIT_TIMEOUT_MS, + outputTimeoutMs: LOCAL_OUTPUT_TIMEOUT_MS, + }); + return { + run, + gateway, + mockModel, + cleanup: async () => { + run.dispose(); + await gateway.cleanup(); + await mockModel.stop(); + await rm(tempDir, { recursive: true, force: true }); + }, + }; + } catch (error) { + await gateway.cleanup(); + await mockModel.stop(); + await rm(tempDir, { recursive: true, force: true }); + throw error; + } +} + +describe("TUI PTY real backends", () => { afterEach(async () => { for (const run of activeRuns.splice(0)) { run.dispose(); @@ -319,4 +413,173 @@ describe("TUI PTY local mode", () => { }, LOCAL_TEST_TIMEOUT_MS, ); + + it( + "forwards an active-run prompt through the real Gateway followup queue", + async () => { + const fixture = await startGatewayModeTui({ queueMode: "followup" }); + try { + await fixture.run.waitForOutput("gateway connected", LOCAL_STARTUP_TIMEOUT_MS); + await fixture.run.write("slow first turn\r"); + await waitFor({ + timeoutMs: LOCAL_OUTPUT_TIMEOUT_MS, + read: () => (fixture.mockModel.requests().length === 1 ? true : null), + onTimeout: () => + new Error(`first prompt did not reach the model\n${fixture.run.output()}`), + }); + + await fixture.run.write("queued followup turn\r"); + await waitFor({ + timeoutMs: LOCAL_OUTPUT_TIMEOUT_MS, + read: () => (fixture.mockModel.requests().length === 2 ? true : null), + onTimeout: () => + new Error( + `queued prompt did not reach the model\nrequests=${JSON.stringify( + fixture.mockModel.requests(), + null, + 2, + )}\n${fixture.gateway.logs()}\n${fixture.run.output()}`, + ), + }); + await fixture.run.waitForOutput("FOLLOWUP_RUN_COMPLETE"); + + await fixture.run.write("turn after queued followup\r"); + await waitFor({ + timeoutMs: LOCAL_OUTPUT_TIMEOUT_MS, + read: () => (fixture.mockModel.requests().length === 3 ? true : null), + onTimeout: () => + new Error( + `TUI stayed blocked after queued followup\nrequests=${JSON.stringify( + fixture.mockModel.requests(), + null, + 2, + )}\n${fixture.gateway.logs()}\n${fixture.run.output()}`, + ), + }); + expect(JSON.stringify(fixture.mockModel.requests()[2]?.body)).toContain( + "turn after queued followup", + ); + + await fixture.run.write("/exit\r", { delay: false }); + expect((await fixture.run.waitForExit()).exitCode).toBe(0); + } finally { + await fixture.cleanup(); + } + }, + LOCAL_TEST_TIMEOUT_MS, + ); + + it( + "cancels an admitted followup with Esc before it reaches the model", + async () => { + const fixture = await startGatewayModeTui({ + queueMode: "followup", + firstResponseDelayMs: 3_000, + }); + try { + await fixture.run.waitForOutput("gateway connected", LOCAL_STARTUP_TIMEOUT_MS); + await fixture.run.write("slow turn to abort\r"); + await waitFor({ + timeoutMs: LOCAL_OUTPUT_TIMEOUT_MS, + read: () => (fixture.mockModel.requests().length === 1 ? true : null), + onTimeout: () => + new Error(`first prompt did not reach the model\n${fixture.run.output()}`), + }); + await fixture.run.write("must never reach model\r"); + await sleep(150); + await fixture.run.write("\u001b", { delay: false }); + await fixture.run.waitForOutput("aborted"); + await sleep(3_250); + + expect(fixture.mockModel.requests()).toHaveLength(1); + expect(fixture.run.output()).not.toContain("FOLLOWUP_RUN_COMPLETE"); + + await fixture.run.write("/exit\r", { delay: false }); + expect((await fixture.run.waitForExit()).exitCode).toBe(0); + } finally { + await fixture.cleanup(); + } + }, + LOCAL_TEST_TIMEOUT_MS, + ); + + it( + "collects two TUI-client prompts into one real Gateway followup turn", + async () => { + const fixture = await startGatewayModeTui({ + queueMode: "collect", + firstResponseDelayMs: 4_000, + queueDebounceMs: 1_000, + }); + const queueClient = new GatewayChatClient({ + url: fixture.gateway.url, + token: fixture.gateway.gatewayToken, + allowInsecureLocalOperatorUi: false, + }); + try { + let queueClientConnected = false; + queueClient.onConnected = () => { + queueClientConnected = true; + }; + queueClient.start(); + await fixture.run.waitForOutput("gateway connected", LOCAL_STARTUP_TIMEOUT_MS); + await waitFor({ + timeoutMs: LOCAL_STARTUP_TIMEOUT_MS, + read: () => (queueClientConnected ? true : null), + onTimeout: () => new Error("TUI Gateway client did not connect"), + }); + await fixture.run.write("/queue collect debounce:1s\r", { delay: false }); + await fixture.run.waitForOutput("Queue mode set to collect."); + await fixture.run.write("slow collect parent\r"); + await waitFor({ + timeoutMs: LOCAL_OUTPUT_TIMEOUT_MS, + read: () => (fixture.mockModel.requests().length === 1 ? true : null), + onTimeout: () => + new Error(`first prompt did not reach the model\n${fixture.run.output()}`), + }); + const alphaSend = queueClient.sendChat({ + sessionKey: "agent:main:main", + message: "collect prompt alpha", + runId: "collect-alpha", + }); + await sleep(100); + const betaSend = queueClient.sendChat({ + sessionKey: "agent:main:main", + message: "collect prompt beta", + runId: "collect-beta", + }); + const sendResults = await Promise.all([alphaSend, betaSend]); + expect(sendResults.map((result) => result.status)).toEqual(["started", "started"]); + await waitFor({ + timeoutMs: LOCAL_OUTPUT_TIMEOUT_MS, + read: () => (fixture.mockModel.requests().length === 2 ? true : null), + onTimeout: () => + new Error( + `collected prompt did not reach the model\n${fixture.gateway.logs()}\n${fixture.run.output()}`, + ), + }); + await sleep(1_000); + + const requests = fixture.mockModel.requests(); + expect( + requests, + `collect emitted ${requests.length} model requests\n${JSON.stringify( + requests.map((request) => request.body.input), + null, + 2, + )}\n${fixture.gateway.logs()}`, + ).toHaveLength(2); + const collectedBody = JSON.stringify(fixture.mockModel.requests()[1]?.body); + expect(collectedBody).toContain("collect prompt alpha"); + expect(collectedBody).toContain("collect prompt beta"); + + await fixture.run.write("/exit\r", { delay: false }); + expect((await fixture.run.waitForExit()).exitCode).toBe(0); + } finally { + queueClient.stop(); + await fixture.cleanup(); + } + }, + LOCAL_TEST_TIMEOUT_MS, + ); }); diff --git a/src/tui/tui-session-actions.test.ts b/src/tui/tui-session-actions.test.ts index 7b89db03ad6..aecc6da0e56 100644 --- a/src/tui/tui-session-actions.test.ts +++ b/src/tui/tui-session-actions.test.ts @@ -899,7 +899,7 @@ describe("tui session actions", () => { expect(addSystem).not.toHaveBeenCalled(); }); - it("aborts the in-flight runId when only pendingChatRunId is set", async () => { + it("uses session-scoped abort when only pendingChatRunId is set", async () => { const abortChat = vi.fn().mockResolvedValue({ ok: true, aborted: true }); const addSystem = vi.fn(); const setActivityStatus = vi.fn(); @@ -932,7 +932,6 @@ describe("tui session actions", () => { expect(abortChat).toHaveBeenCalledWith({ sessionKey: "agent:main:main", - runId: "run-pending", }); expect(addSystem).not.toHaveBeenCalledWith("no active run"); expect(state.pendingChatRunId).toBeNull(); @@ -991,6 +990,73 @@ describe("tui session actions", () => { expect(dropPendingUser).not.toHaveBeenCalled(); }); + it("drops terminalized queued rows returned by session abort", async () => { + const abortChat = vi.fn().mockResolvedValue({ + ok: true, + aborted: true, + runIds: ["run-active", "run-queued-terminal"], + }); + const dropPendingUser = vi.fn(); + const state = createBaseState({ + activeChatRunId: "run-active", + pendingChatRunId: null, + pendingSubmitDraft: null, + }); + + const { abortActive } = createTestSessionActions({ + client: { listSessions: vi.fn(), abortChat } as unknown as TuiBackend, + chatLog: { + addSystem: vi.fn(), + clearAll: vi.fn(), + dropPendingUser, + } as unknown as import("./components/chat-log.js").ChatLog, + state, + }); + + await abortActive(); + + expect(dropPendingUser).toHaveBeenCalledTimes(1); + expect(dropPendingUser).toHaveBeenCalledWith("run-queued-terminal"); + }); + + it("drops a queued row that terminalizes while session abort is pending", async () => { + let resolveAbort: + | ((value: { ok: boolean; aborted: boolean; runIds: string[] }) => void) + | undefined; + const abortChat = vi.fn().mockImplementation( + () => + new Promise<{ ok: boolean; aborted: boolean; runIds: string[] }>((resolve) => { + resolveAbort = resolve; + }), + ); + const dropPendingUser = vi.fn(); + const state = createBaseState({ + activeChatRunId: "run-active", + pendingChatRunId: "run-queued", + pendingOptimisticUserMessage: true, + pendingSubmitDraft: { runId: "run-queued", text: "queued" }, + }); + const { abortActive } = createTestSessionActions({ + client: { listSessions: vi.fn(), abortChat } as unknown as TuiBackend, + chatLog: { + addSystem: vi.fn(), + clearAll: vi.fn(), + dropPendingUser, + } as unknown as import("./components/chat-log.js").ChatLog, + state, + }); + + const pendingAbort = abortActive(); + await vi.waitFor(() => expect(abortChat).toHaveBeenCalledOnce()); + state.pendingChatRunId = null; + state.pendingSubmitDraft = null; + resolveAbort?.({ ok: true, aborted: true, runIds: ["run-active", "run-queued"] }); + await pendingAbort; + + expect(dropPendingUser).toHaveBeenCalledTimes(1); + expect(dropPendingUser).toHaveBeenCalledWith("run-queued"); + }); + it("passes the selected agent when aborting selected global runs", async () => { const abortChat = vi.fn().mockResolvedValue({ ok: true, aborted: true }); const state = createBaseState({ @@ -1009,15 +1075,16 @@ describe("tui session actions", () => { expect(abortChat).toHaveBeenCalledWith({ sessionKey: "global", agentId: "work", - runId: "run-work-global", }); }); it("coalesces repeated no-active-run abort notices", async () => { + const abortChat = vi.fn().mockResolvedValue({ ok: true, aborted: false }); const addSystem = vi.fn(); const requestRender = vi.fn(); const { abortActive } = createTestSessionActions({ + client: { listSessions: vi.fn(), abortChat } as unknown as TuiBackend, chatLog: { addSystem, clearAll: vi.fn(), @@ -1033,6 +1100,33 @@ describe("tui session actions", () => { expect(requestRender).toHaveBeenCalledOnce(); }); + it("preserves pending UI state when session abort finds no backend run", async () => { + const abortChat = vi.fn().mockResolvedValue({ ok: true, aborted: false }); + const dropPendingUser = vi.fn(); + const state = createBaseState({ + pendingChatRunId: "run-pending", + pendingOptimisticUserMessage: true, + pendingSubmitDraft: { runId: "run-pending", text: "hello" }, + }); + + const { abortActive } = createTestSessionActions({ + client: { listSessions: vi.fn(), abortChat } as unknown as TuiBackend, + chatLog: { + addSystem: vi.fn(), + clearAll: vi.fn(), + dropPendingUser, + } as unknown as import("./components/chat-log.js").ChatLog, + state, + }); + + await abortActive(); + + expect(state.pendingChatRunId).toBe("run-pending"); + expect(state.pendingOptimisticUserMessage).toBe(true); + expect(state.pendingSubmitDraft).toEqual({ runId: "run-pending", text: "hello" }); + expect(dropPendingUser).not.toHaveBeenCalled(); + }); + it("does not abort local post-turn maintenance while finishing context", async () => { const abortChat = vi.fn().mockResolvedValue({ ok: true, aborted: true }); const addSystem = vi.fn(); @@ -1082,9 +1176,9 @@ describe("tui session actions", () => { await abortActive({ preferActive: true }); + // Session-scoped abort: Gateway cancels authorized queued turns first, then active. expect(abortChat).toHaveBeenCalledWith({ sessionKey: "agent:main:main", - runId: "run-finishing", }); expect(setActivityStatus).toHaveBeenCalledWith("aborted"); }); @@ -1110,7 +1204,6 @@ describe("tui session actions", () => { expect(abortChat).toHaveBeenCalledWith({ sessionKey: "agent:main:main", - runId: "run-queued", }); expect(state.pendingChatRunId).toBeNull(); expect(state.pendingOptimisticUserMessage).toBe(false); @@ -1137,7 +1230,6 @@ describe("tui session actions", () => { expect(abortChat).toHaveBeenCalledWith({ sessionKey: "agent:main:main", - runId: "run-queued", }); expect(state.pendingChatRunId).toBeNull(); expect(setActivityStatus).toHaveBeenCalledWith("aborted"); @@ -1161,13 +1253,10 @@ describe("tui session actions", () => { await abortActive({ preferActive: true }); - expect(abortChat).toHaveBeenNthCalledWith(1, { + // One session abort covers queued + active with Gateway-owned cancel order. + expect(abortChat).toHaveBeenCalledTimes(1); + expect(abortChat).toHaveBeenCalledWith({ sessionKey: "agent:main:main", - runId: "run-queued", - }); - expect(abortChat).toHaveBeenNthCalledWith(2, { - sessionKey: "agent:main:main", - runId: "run-active", }); expect(state.pendingChatRunId).toBeNull(); expect(setActivityStatus).toHaveBeenCalledWith("aborted"); diff --git a/src/tui/tui-session-actions.ts b/src/tui/tui-session-actions.ts index 80e971c0f82..339ccd3ec4f 100644 --- a/src/tui/tui-session-actions.ts +++ b/src/tui/tui-session-actions.ts @@ -625,30 +625,30 @@ export function createSessionActions(context: SessionActionContext) { tui.requestRender(); return; } - const runIds = - params?.preferActive && state.activeChatRunId && state.pendingChatRunId - ? [state.pendingChatRunId, state.activeChatRunId] - : [ - !params?.preferActive && state.activeChatRunId && state.pendingChatRunId - ? state.pendingChatRunId - : (state.activeChatRunId ?? state.pendingChatRunId ?? null), - ].filter((runId) => runId !== null); - if (runIds.length === 0) { - chatLog.addSystem("no active run", { coalesceConsecutive: true }); - tui.requestRender(); - return; - } - const abortsPendingRun = Boolean( - state.pendingChatRunId && runIds.includes(state.pendingChatRunId), - ); + const abortsPendingRun = Boolean(state.pendingChatRunId); + const activeRunId = state.activeChatRunId; const pendingRunId = state.pendingChatRunId; + const sessionAbortParams = { + sessionKey: state.currentSessionKey, + ...(state.currentSessionKey === "global" ? { agentId: state.currentAgentId } : {}), + }; try { - for (const runId of runIds) { - await client.abortChat({ - sessionKey: state.currentSessionKey, - ...(state.currentSessionKey === "global" ? { agentId: state.currentAgentId } : {}), - runId, - }); + // Session-scoped abort is the only reliable TUI stop contract: queued + // chat.send calls can terminalize before the queue drains, so their run + // ids may no longer exist in local UI state. + const result = await client.abortChat(sessionAbortParams); + if (!result.aborted) { + chatLog.addSystem("no active run", { coalesceConsecutive: true }); + tui.requestRender(); + return; + } + for (const runId of result.runIds ?? []) { + const stillTracked = state.activeChatRunId === runId || state.pendingChatRunId === runId; + // The active prompt is already persisted. Pending/queued prompts may + // terminalize while the RPC is in flight, so inspect their live state. + if (runId !== activeRunId && !stillTracked) { + chatLog.dropPendingUser(runId); + } } state.pendingChatRunId = null; if (abortsPendingRun) { diff --git a/src/tui/tui-types.ts b/src/tui/tui-types.ts index 9b81c2fd522..91031e58620 100644 --- a/src/tui/tui-types.ts +++ b/src/tui/tui-types.ts @@ -1,6 +1,6 @@ +import type { FastMode } from "@openclaw/normalization-core/string-coerce"; // Defines shared TUI state, backend, and event types. import type { SessionGoal } from "../config/sessions/types.js"; -import type { FastMode } from "@openclaw/normalization-core/string-coerce"; export type TuiOptions = { local?: boolean; diff --git a/src/tui/tui.test.ts b/src/tui/tui.test.ts index 1d728a1047e..e67114d3e47 100644 --- a/src/tui/tui.test.ts +++ b/src/tui/tui.test.ts @@ -131,38 +131,26 @@ describe("canSubmitTuiChatMessage", () => { expect(canSubmitTuiChatMessage({})).toBe(true); }); - it("allows local submit while a run is active", () => { + it("allows submit while a run is active so the backend owns queue policy", () => { expect( canSubmitTuiChatMessage({ - local: true, activeChatRunId: "run-active", }), ).toBe(true); }); - it("blocks gateway submit while a run is active", () => { + it("allows stop text while a run is active", () => { expect( canSubmitTuiChatMessage({ - local: false, - activeChatRunId: "run-active", - }), - ).toBe(false); - }); - - it("allows gateway stop text while a run is active", () => { - expect( - canSubmitTuiChatMessage({ - local: false, activeChatRunId: "run-active", message: "please stop", }), ).toBe(true); }); - it("allows local stop text while a queued run is pending", () => { + it("allows stop text while a queued run is pending", () => { expect( canSubmitTuiChatMessage({ - local: true, activeChatRunId: "run-active", pendingChatRunId: "run-queued", message: "please stop", @@ -185,6 +173,15 @@ describe("canSubmitTuiChatMessage", () => { }), ).toBe(false); }); + + it("blocks submit while optimistic state is pending during an active run", () => { + expect( + canSubmitTuiChatMessage({ + activeChatRunId: "run-active", + pendingOptimisticUserMessage: true, + }), + ).toBe(false); + }); }); describe("isTuiBusyActivityStatus", () => { @@ -241,9 +238,7 @@ describe("resolveTuiShutdownHardExitMs", () => { it("clamps oversized local run shutdown grace values", () => { withEnv({ OPENCLAW_TUI_LOCAL_RUN_SHUTDOWN_GRACE_MS: String(Number.MAX_SAFE_INTEGER) }, () => { - expect(resolveTuiShutdownHardExitMs({ localMode: true })).toBe( - MAX_TIMER_TIMEOUT_MS + 2000, - ); + expect(resolveTuiShutdownHardExitMs({ localMode: true })).toBe(MAX_TIMER_TIMEOUT_MS + 2000); }); }); }); diff --git a/src/tui/tui.ts b/src/tui/tui.ts index ff7dadee137..3d87d442222 100644 --- a/src/tui/tui.ts +++ b/src/tui/tui.ts @@ -403,7 +403,6 @@ export async function drainAndStopTuiSafely(tui: DrainableTui): Promise { } export function canSubmitTuiChatMessage(params: { - local?: boolean; activeChatRunId?: string | null; pendingChatRunId?: string | null; pendingOptimisticUserMessage?: boolean; @@ -413,11 +412,7 @@ export function canSubmitTuiChatMessage(params: { if (stopText && (params.activeChatRunId || params.pendingChatRunId)) { return true; } - const pending = Boolean(params.pendingChatRunId) || params.pendingOptimisticUserMessage === true; - if (!params.local && params.activeChatRunId) { - return false; - } - return !pending; + return !params.pendingChatRunId && params.pendingOptimisticUserMessage !== true; } const TUI_BUSY_ACTIVITY_STATUSES = new Set([ @@ -1428,7 +1423,6 @@ export async function runTui(opts: RunTuiOptions): Promise { updateAutocompleteProvider(); const canSubmitChatMessage = (message: string) => canSubmitTuiChatMessage({ - local: isLocalMode, activeChatRunId: state.activeChatRunId, pendingChatRunId: state.pendingChatRunId, pendingOptimisticUserMessage: state.pendingOptimisticUserMessage, diff --git a/test/helpers/openclaw-test-instance.ts b/test/helpers/openclaw-test-instance.ts index 48c070f718f..49db7580913 100644 --- a/test/helpers/openclaw-test-instance.ts +++ b/test/helpers/openclaw-test-instance.ts @@ -1,9 +1,10 @@ // OpenClaw test instance helper spawns isolated OpenClaw processes. -import { type ChildProcessWithoutNullStreams, spawn } from "node:child_process"; +import { type ChildProcessByStdio, spawn } from "node:child_process"; import { randomUUID } from "node:crypto"; import fs from "node:fs/promises"; import net from "node:net"; import path from "node:path"; +import type { Readable } from "node:stream"; import { BUILD_STAMP_FILE, RUNTIME_POSTBUILD_STAMP_FILE, @@ -36,6 +37,8 @@ export type OpenClawTestInstanceCommandResult = { stderr: string; }; +type OpenClawTestProcess = ChildProcessByStdio; + export type OpenClawTestInstance = { name: string; port: number; @@ -48,7 +51,7 @@ export type OpenClawTestInstance = { state: OpenClawTestState; stdout: string[]; stderr: string[]; - child?: ChildProcessWithoutNullStreams; + child?: OpenClawTestProcess; env: NodeJS.ProcessEnv; entrypoint: () => Promise; cli: ( @@ -73,7 +76,7 @@ type BoundedStringLog = string[] & { truncated?: boolean; }; -type OpenClawTestChildProcess = Pick; +type OpenClawTestChildProcess = Pick; function createBoundedStringLog(): string[] { const log = [] as BoundedStringLog; @@ -210,7 +213,7 @@ const getFreePort = async () => { }; async function waitForPortOpen( - proc: ChildProcessWithoutNullStreams, + proc: OpenClawTestProcess, chunksOut: string[], chunksErr: string[], port: number, @@ -250,10 +253,7 @@ async function waitForPortOpen( ); } -async function waitForGatewayExit( - child: ChildProcessWithoutNullStreams, - timeoutMs: number, -): Promise { +async function waitForGatewayExit(child: OpenClawTestProcess, timeoutMs: number): Promise { return await Promise.race([ new Promise((resolve) => { if (child.exitCode !== null || child.signalCode !== null) { @@ -266,7 +266,7 @@ async function waitForGatewayExit( ]); } -function hasChildExited(child: Pick) { +function hasChildExited(child: Pick) { return child.exitCode !== null || child.signalCode !== null; } @@ -354,7 +354,7 @@ export async function createOpenClawTestInstance( stateEnv: state.env, extraEnv: options.env ?? {}, }); - let child: ChildProcessWithoutNullStreams | undefined; + let child: OpenClawTestProcess | undefined; let cleaned = false; const instance: OpenClawTestInstance = { @@ -518,7 +518,8 @@ function shouldUseOpenClawTestProcessGroup(): boolean { function signalOpenClawTestProcess( child: OpenClawTestChildProcess, signal: NodeJS.Signals, - killProcess: (pid: number, signal: NodeJS.Signals) => boolean = process.kill, + killProcess: (pid: number, signal: NodeJS.Signals) => boolean = (pid, nextSignal) => + process.kill(pid, nextSignal), ): void { if (shouldUseOpenClawTestProcessGroup() && typeof child.pid === "number") { try {