From 414ecd2b96899d955f10e93ca166915bdd9d24ad Mon Sep 17 00:00:00 2001 From: Eva Date: Fri, 3 Jul 2026 14:07:54 +0200 Subject: [PATCH] Preserve Codex output after missing turn completion (#99217) * Preserve Codex output after missing turn completion * fix: narrow Codex completion-timeout output recovery * test(codex): narrow binding path guard * fix: narrow Codex completion-timeout output recovery * test(codex): narrow binding path guard * fix(codex): preserve id-less post-tool replies --------- Co-authored-by: Sedrak-Hovhannisyan <264150421+Sedrak-Hovhannisyan@users.noreply.github.com> Co-authored-by: Eva Co-authored-by: Jason (Json) <263060202+fuller-stack-dev@users.noreply.github.com> --- docs/plugins/codex-harness.md | 13 +- .../app-server/attempt-turn-watches.test.ts | 40 + .../src/app-server/attempt-turn-watches.ts | 21 +- .../codex/src/app-server/event-projector.ts | 184 ++- .../src/app-server/run-attempt.hooks.test.ts | 403 ++++++ .../codex/src/app-server/run-attempt.ts | 467 +++++-- .../run-attempt.turn-watches.test.ts | 1230 ++++++++++++++++- .../agent-command.live-model-switch.test.ts | 31 +- src/agents/agent-command.ts | 6 +- src/agents/embedded-agent-runner/run-state.ts | 10 + .../run.codex-app-server-recovery.test.ts | 176 ++- src/agents/embedded-agent-runner/run.ts | 36 +- .../embedded-agent-runner/run/attempt.ts | 1 + .../run/codex-app-server-recovery.ts | 12 +- src/agents/embedded-agent-runner/runs.test.ts | 203 +++ src/agents/embedded-agent-runner/runs.ts | 111 +- .../model-fallback.run-embedded.e2e.test.ts | 2 + src/agents/model-fallback.test.ts | 24 +- src/agents/model-fallback.ts | 5 +- src/agents/run-termination.test.ts | 13 + src/agents/run-termination.ts | 14 + src/agents/subagent-control.runtime.ts | 2 +- src/agents/subagent-control.test.ts | 105 ++ src/agents/subagent-control.ts | 33 +- src/auto-reply/reply/abort.runtime-types.ts | 6 +- src/auto-reply/reply/abort.test.ts | 50 +- src/auto-reply/reply/abort.ts | 85 +- ...agent-runner-direct-runtime-config.test.ts | 28 + .../reply/agent-runner-execution.test.ts | 225 ++- .../reply/agent-runner-execution.ts | 127 +- ...nner-memory.preflight-stale-tokens.test.ts | 13 +- .../reply/agent-runner-memory.test.ts | 25 +- src/auto-reply/reply/agent-runner-memory.ts | 1 + ...t-runner.final-media-runreplyagent.test.ts | 1 + .../reply/agent-runner.media-paths.test.ts | 1 + .../agent-runner.misc.runreplyagent.test.ts | 77 ++ src/auto-reply/reply/agent-runner.ts | 5 + .../reply/commands-abort-trigger.test.ts | 32 +- src/auto-reply/reply/commands-compact.test.ts | 3 +- .../reply/commands-session-abort.ts | 29 +- .../reply/commands-stop-target.test.ts | 31 +- .../reply/dispatch-from-config.test.ts | 31 +- src/auto-reply/reply/dispatch-from-config.ts | 9 +- src/auto-reply/reply/followup-runner.test.ts | 143 ++ src/auto-reply/reply/followup-runner.ts | 23 + .../reply/reply-run-registry.test.ts | 208 +++ src/auto-reply/reply/reply-run-registry.ts | 120 +- src/gateway/chat-abort.test.ts | 25 + src/gateway/chat-abort.ts | 47 +- src/gateway/server-close.test.ts | 33 + src/gateway/server-close.ts | 7 +- src/gateway/server-maintenance.ts | 5 +- src/gateway/server-methods/agent.ts | 15 +- .../chat.abort-persistence.test.ts | 38 + src/gateway/server-methods/chat.ts | 5 +- src/gateway/server-runtime-subscriptions.ts | 18 +- 56 files changed, 4324 insertions(+), 284 deletions(-) diff --git a/docs/plugins/codex-harness.md b/docs/plugins/codex-harness.md index e89d2fa94e5d..a808bd008195 100644 --- a/docs/plugins/codex-harness.md +++ b/docs/plugins/codex-harness.md @@ -662,9 +662,16 @@ guard instead of releasing the session lane immediately. Only final/non-commentary completed `agentMessage` items and pre-tool raw assistant completions arm the assistant-output release: if Codex then goes quiet without `turn/completed`, OpenClaw best-effort interrupts the native turn and -releases the session lane. Replay-safe stdio app-server failures, including -turn-completion idle timeouts without assistant, tool, active-item, or -side-effect evidence, are retried once on a fresh app-server attempt. Unsafe +releases the session lane. If another turn watch wins that release race, +OpenClaw still accepts the completed final assistant item once no native +request, item, or dynamic tool completion remains active and the +assistant-output release still belongs to the latest completed item, with no +later item completion. This can preserve the final answer after completed tool +work without replaying the turn. Partial assistant deltas, stale earlier +replies, and empty later completions do not qualify. Replay-safe stdio +app-server failures, +including turn-completion idle timeouts without assistant, tool, active-item, +or side-effect evidence, are retried once on a fresh app-server attempt. Unsafe timeouts still retire the stuck app-server client and release the OpenClaw session lane. They also clear the stale native thread binding instead of being replayed automatically. Completion-watch timeouts surface Codex-specific timeout diff --git a/extensions/codex/src/app-server/attempt-turn-watches.test.ts b/extensions/codex/src/app-server/attempt-turn-watches.test.ts index 2f9762dd5358..9545b461cb14 100644 --- a/extensions/codex/src/app-server/attempt-turn-watches.test.ts +++ b/extensions/codex/src/app-server/attempt-turn-watches.test.ts @@ -25,6 +25,8 @@ describe("Codex app-server attempt turn watches", () => { let activeRequests = 0; let activeItems = 0; let activeCompletionBlockers = 0; + let activeFinalizationHooks = 0; + let canReleaseAssistantCompletionIdle = true; const interrupts: Array> = []; const timeouts: Array> = []; const events: Array<{ name: string; fields: Record }> = []; @@ -39,6 +41,8 @@ describe("Codex app-server attempt turn watches", () => { getActiveAppServerTurnRequests: () => activeRequests, getActiveTurnItemCount: () => activeItems, getActiveCompletionBlockerItemCount: () => activeCompletionBlockers, + getActiveFinalizationHookCount: () => activeFinalizationHooks, + canReleaseAssistantCompletionIdle: () => canReleaseAssistantCompletionIdle, turnCompletionIdleTimeoutMs: 10, turnAssistantCompletionIdleTimeoutMs: 10, turnAttemptIdleTimeoutMs: 10, @@ -75,6 +79,12 @@ describe("Codex app-server attempt turn watches", () => { set activeCompletionBlockers(value: number) { activeCompletionBlockers = value; }, + set activeFinalizationHooks(value: number) { + activeFinalizationHooks = value; + }, + set canReleaseAssistantCompletionIdle(value: boolean) { + canReleaseAssistantCompletionIdle = value; + }, interrupts, timeouts, events, @@ -198,6 +208,19 @@ describe("Codex app-server attempt turn watches", () => { expect(harness.events[0]?.name).toBe("turn.assistant_completion_idle_release"); }); + it("does not release when a later completed item supersedes the assistant", () => { + const harness = createController(); + + harness.controller.armAssistantCompletionIdleWatch({ method: "item/completed" }); + harness.canReleaseAssistantCompletionIdle = false; + vi.advanceTimersByTime(10); + + expect(harness.completed).toBe(false); + expect(harness.controller.isAssistantCompletionIdleWatchArmed()).toBe(false); + expect(harness.interrupts).toEqual([]); + expect(harness.events).toEqual([]); + }); + it("waits for active turn items before assistant idle release", () => { const harness = createController(); harness.activeItems = 1; @@ -212,6 +235,23 @@ describe("Codex app-server attempt turn watches", () => { expect(harness.completed).toBe(true); }); + it("waits for active finalization hooks before assistant idle release", () => { + const harness = createController(); + + harness.controller.armAssistantCompletionIdleWatch(); + harness.activeFinalizationHooks = 1; + vi.advanceTimersByTime(10); + + expect(harness.completed).toBe(false); + expect(harness.interrupts).toEqual([]); + + harness.activeFinalizationHooks = 0; + harness.controller.armAssistantCompletionIdleWatch(); + vi.advanceTimersByTime(10); + + expect(harness.completed).toBe(true); + }); + it("records attempt progress activity separately from completion-only activity", () => { const harness = createController(); diff --git a/extensions/codex/src/app-server/attempt-turn-watches.ts b/extensions/codex/src/app-server/attempt-turn-watches.ts index 943b204d9bed..1200a6561de0 100644 --- a/extensions/codex/src/app-server/attempt-turn-watches.ts +++ b/extensions/codex/src/app-server/attempt-turn-watches.ts @@ -37,6 +37,8 @@ export function createCodexAttemptTurnWatchController(params: { getActiveAppServerTurnRequests: () => number; getActiveTurnItemCount: () => number; getActiveCompletionBlockerItemCount: () => number; + getActiveFinalizationHookCount: () => number; + canReleaseAssistantCompletionIdle: () => boolean; turnCompletionIdleTimeoutMs: number; turnAssistantCompletionIdleTimeoutMs: number; turnAttemptIdleTimeoutMs: number; @@ -136,7 +138,12 @@ export function createCodexAttemptTurnWatchController(params: { function scheduleAssistantCompletionIdleWatch() { clearAssistantCompletionIdleTimer(); - if (params.isCompleted() || params.signal.aborted || !assistantCompletionIdleWatchArmed) { + if ( + params.isCompleted() || + params.signal.aborted || + !assistantCompletionIdleWatchArmed || + params.getActiveFinalizationHookCount() > 0 + ) { return; } const elapsedMs = Math.max(0, Date.now() - assistantCompletionLastActivityAt); @@ -216,10 +223,20 @@ export function createCodexAttemptTurnWatchController(params: { if (params.isCompleted() || params.signal.aborted || !assistantCompletionIdleWatchArmed) { return; } - if (params.getActiveAppServerTurnRequests() > 0 || params.getActiveTurnItemCount() > 0) { + if ( + params.getActiveAppServerTurnRequests() > 0 || + params.getActiveTurnItemCount() > 0 || + params.getActiveFinalizationHookCount() > 0 + ) { scheduleAssistantCompletionIdleWatch(); return; } + if (!params.canReleaseAssistantCompletionIdle()) { + assistantCompletionIdleWatchArmed = false; + assistantCompletionLastActivityDetails = undefined; + clearAssistantCompletionIdleTimer(); + return; + } const idleMs = Math.max(0, Date.now() - assistantCompletionLastActivityAt); if (idleMs < turnAssistantCompletionIdleTimeoutMs) { scheduleAssistantCompletionIdleWatch(); diff --git a/extensions/codex/src/app-server/event-projector.ts b/extensions/codex/src/app-server/event-projector.ts index e735a1093597..f7d21ee6122b 100644 --- a/extensions/codex/src/app-server/event-projector.ts +++ b/extensions/codex/src/app-server/event-projector.ts @@ -150,6 +150,13 @@ export class CodexAppServerEventProjector { private readonly assistantTextByItem = new Map(); private readonly assistantItemOrder: string[] = []; private readonly assistantPhaseByItem = new Map(); + private latestCompletedItemId: string | undefined; + private latestCompletedTerminalAssistantItemId: string | undefined; + private latestTerminalAssistantCandidateItemId: string | undefined; + private latestTerminalAssistantCandidateSuperseded = false; + private latestTerminalAssistantCandidateCanReleaseAfterToolHandoff = false; + private terminalAssistantCandidateEarlierActiveItemIds = new Set(); + private pendingRawTerminalAssistantEchoItemId: string | undefined; private readonly lastCommentaryProgressTextByItem = new Map(); // Codex emits each typed item completion before its matching raw response item. // Pair by protocol order because contributors may rewrite only the typed text. @@ -220,8 +227,57 @@ export class CodexAppServerEventProjector { } hasCompletedTerminalAssistantText(): boolean { + const latestCompletedItemId = this.latestCompletedTerminalAssistantItemId; + if (!latestCompletedItemId) { + return false; + } const finalItem = this.resolveFinalAssistantTextItem(); - return finalItem !== undefined && this.completedItemIds.has(finalItem.itemId); + return ( + this.latestCompletedItemId === latestCompletedItemId && + finalItem?.itemId === latestCompletedItemId && + this.completedItemIds.has(latestCompletedItemId) + ); + } + + getLatestTerminalAssistantCandidate(): { itemId: string; hasText: boolean } | undefined { + const itemId = this.latestTerminalAssistantCandidateItemId; + if (!itemId) { + return undefined; + } + const text = this.assistantTextByItem.get(itemId)?.trim(); + return { + itemId, + hasText: Boolean(text && !this.toolProgressTexts.has(text)), + }; + } + + hasLatestTerminalAssistantCandidateText(): boolean { + return ( + !this.latestTerminalAssistantCandidateSuperseded && + this.getLatestTerminalAssistantCandidate()?.hasText === true + ); + } + + canReleaseLatestTerminalAssistantAfterToolHandoff(): boolean { + return ( + this.latestTerminalAssistantCandidateCanReleaseAfterToolHandoff && + this.hasLatestTerminalAssistantCandidateText() + ); + } + + /** Restores a completed final item after only the enclosing turn timeout fired. */ + recoverCompletedTerminalAssistantAfterTurnWatchTimeout(): boolean { + if ( + !this.aborted || + this.promptError !== "codex app-server attempt timed out" || + !this.hasCompletedTerminalAssistantText() + ) { + return false; + } + this.aborted = false; + this.promptError = undefined; + this.promptErrorSource = null; + return true; } /** Resolves the shared model-order position for a native tool item. */ @@ -511,11 +567,18 @@ export class CodexAppServerEventProjector { if (!delta) { return; } + if (itemId !== this.pendingRawTerminalAssistantEchoItemId) { + this.pendingRawTerminalAssistantEchoItemId = undefined; + } this.rememberAssistantPhase(readItem(params.item)); const phase = readString(params, "phase"); if (phase) { this.assistantPhaseByItem.set(itemId, phase); } + const isCommentary = this.isCommentaryAssistantItem(itemId); + if (!isCommentary && itemId !== this.latestTerminalAssistantCandidateItemId) { + this.markTerminalAssistantCandidateSupersededBy(); + } if (!this.assistantStarted) { this.assistantStarted = true; await this.params.onAssistantMessageStart?.(); @@ -523,7 +586,7 @@ export class CodexAppServerEventProjector { this.rememberAssistantItem(itemId); const text = `${this.assistantTextByItem.get(itemId) ?? ""}${delta}`; this.assistantTextByItem.set(itemId, text); - if (this.isCommentaryAssistantItem(itemId)) { + if (isCommentary) { this.emitCommentaryProgress({ itemId, text }); } else { const knownFinalAnswer = this.shouldStreamAssistantPartial(itemId); @@ -636,9 +699,24 @@ export class CodexAppServerEventProjector { private async handleItemStarted(params: JsonObject): Promise { const item = readItem(params.item); const itemId = item?.id ?? readString(params, "itemId") ?? readString(params, "id"); + if ( + item?.type === "agentMessage" && + itemId && + itemId !== this.pendingRawTerminalAssistantEchoItemId + ) { + this.pendingRawTerminalAssistantEchoItemId = undefined; + } this.rememberAssistantPhase(item); if (itemId) { this.activeItemIds.add(itemId); + if (itemId !== this.latestTerminalAssistantCandidateItemId) { + this.markTerminalAssistantCandidateSupersededBy(itemId, { + preserveEarlierActiveItem: true, + }); + if (this.latestTerminalAssistantCandidateSuperseded) { + this.pendingRawTerminalAssistantEchoItemId = undefined; + } + } } this.recordNativeToolOutcome(item); if (item?.type === "contextCompaction" && itemId) { @@ -684,11 +762,31 @@ export class CodexAppServerEventProjector { this.recordNativeToolOutcome(item); this.clearTerminalPresentationForNativeItem(item); const itemId = item?.id ?? readString(params, "itemId") ?? readString(params, "id"); + if ( + item?.type === "agentMessage" && + itemId && + itemId !== this.pendingRawTerminalAssistantEchoItemId + ) { + this.pendingRawTerminalAssistantEchoItemId = undefined; + } if (itemId) { this.activeItemIds.delete(itemId); this.completedItemIds.add(itemId); + this.latestCompletedItemId = itemId; } this.rememberAssistantPhase(item); + if (item?.type === "agentMessage" && !this.isCommentaryAssistantItem(item.id)) { + this.latestCompletedTerminalAssistantItemId = item.id; + this.markLatestTerminalAssistantCandidate(item.id); + this.pendingRawTerminalAssistantEchoItemId = item.id; + } else if (itemId) { + this.markTerminalAssistantCandidateSupersededBy(itemId, { + preserveEarlierActiveItem: true, + }); + if (this.latestTerminalAssistantCandidateSuperseded) { + this.pendingRawTerminalAssistantEchoItemId = undefined; + } + } if (item?.type === "agentMessage" && typeof item.text === "string") { this.rememberAssistantItem(item.id); this.assistantTextByItem.set(item.id, item.text); @@ -952,20 +1050,60 @@ export class CodexAppServerEventProjector { if (!item) { return; } + const role = readString(item, "role"); + const phase = readString(item, "phase"); + const rawItemId = readString(item, "id"); + const candidateWasSupersededBeforeRaw = this.latestTerminalAssistantCandidateSuperseded; + const pendingTerminalAssistantEchoItemId = this.pendingRawTerminalAssistantEchoItemId; + const isPendingTerminalAssistantEcho = + role === "assistant" && + phase !== "commentary" && + pendingTerminalAssistantEchoItemId !== undefined && + (rawItemId === undefined || rawItemId === pendingTerminalAssistantEchoItemId); + if (pendingTerminalAssistantEchoItemId !== undefined && !isPendingTerminalAssistantEcho) { + this.pendingRawTerminalAssistantEchoItemId = undefined; + } + if (!isPendingTerminalAssistantEcho) { + this.latestCompletedItemId = undefined; + this.markTerminalAssistantCandidateSupersededBy(rawItemId); + } await this.recordRawGeneratedImageMedia(item); - if (readString(item, "role") !== "assistant") { + if (role !== "assistant") { return; } - const phase = readString(item, "phase"); if (phase === "commentary" && this.pendingRawCommentaryEchoes > 0) { this.pendingRawCommentaryEchoes -= 1; return; } const text = extractRawAssistantText(item); + if (isPendingTerminalAssistantEcho) { + const typedItemId = pendingTerminalAssistantEchoItemId; + this.pendingRawTerminalAssistantEchoItemId = undefined; + // Contributors may rewrite the typed completion without rewriting its raw echo. + if (this.assistantTextByItem.get(typedItemId)?.trim() || !text) { + return; + } + this.rememberAssistantItem(typedItemId); + this.assistantTextByItem.set(typedItemId, text); + return; + } if (!text) { return; } - const itemId = readString(item, "id") ?? `raw-assistant-${this.assistantItemOrder.length + 1}`; + const itemId = rawItemId ?? `raw-assistant-${this.assistantItemOrder.length + 1}`; + const isIdlessTerminalAssistantAfterCompletedWork = + candidateWasSupersededBeforeRaw && + rawItemId === undefined && + pendingTerminalAssistantEchoItemId === undefined && + this.activeItemIds.size === 0; + if ( + phase !== "commentary" && + candidateWasSupersededBeforeRaw && + itemId !== this.streamedPartialAssistantItemId && + !isIdlessTerminalAssistantAfterCompletedWork + ) { + return; + } if (phase) { this.assistantPhaseByItem.set(itemId, phase); } @@ -973,9 +1111,45 @@ export class CodexAppServerEventProjector { this.assistantTextByItem.set(itemId, text); if (phase === "commentary") { this.emitCommentaryProgress({ itemId, text }); + } else { + this.markLatestTerminalAssistantCandidate(itemId, { + canReleaseAfterToolHandoff: isIdlessTerminalAssistantAfterCompletedWork, + }); } } + private markLatestTerminalAssistantCandidate( + itemId: string, + options?: { canReleaseAfterToolHandoff?: boolean }, + ): void { + this.latestTerminalAssistantCandidateItemId = itemId; + this.latestTerminalAssistantCandidateSuperseded = false; + this.latestTerminalAssistantCandidateCanReleaseAfterToolHandoff = + options?.canReleaseAfterToolHandoff === true; + this.terminalAssistantCandidateEarlierActiveItemIds = new Set(this.activeItemIds); + } + + private markTerminalAssistantCandidateSupersededBy( + itemId?: string, + options?: { preserveEarlierActiveItem?: boolean }, + ): void { + if (!this.latestTerminalAssistantCandidateItemId) { + return; + } + // Preserve app-server ordering where an item already active at assistant + // completion reports its delayed completion afterward. Only new work proves + // the candidate stale; origin/main has an integration test for this order. + if (itemId && this.terminalAssistantCandidateEarlierActiveItemIds.has(itemId)) { + if (!options?.preserveEarlierActiveItem) { + this.terminalAssistantCandidateEarlierActiveItemIds.delete(itemId); + } + return; + } + this.latestTerminalAssistantCandidateSuperseded = true; + this.latestTerminalAssistantCandidateCanReleaseAfterToolHandoff = false; + this.terminalAssistantCandidateEarlierActiveItemIds.clear(); + } + private recordNativeGeneratedMedia(item: CodexThreadItem | undefined): void { if (item?.type !== "imageGeneration") { return; diff --git a/extensions/codex/src/app-server/run-attempt.hooks.test.ts b/extensions/codex/src/app-server/run-attempt.hooks.test.ts index 388a04935bfc..f94c2519416c 100644 --- a/extensions/codex/src/app-server/run-attempt.hooks.test.ts +++ b/extensions/codex/src/app-server/run-attempt.hooks.test.ts @@ -1,8 +1,10 @@ // Codex tests cover run attempt.hooks plugin behavior. +import fs from "node:fs/promises"; import path from "node:path"; import { abortAgentHarnessRun, onAgentEvent, + resolveActiveEmbeddedRunSessionId, type AgentEventPayload, } from "openclaw/plugin-sdk/agent-harness-runtime"; import { SessionManager } from "openclaw/plugin-sdk/agent-sessions"; @@ -33,6 +35,11 @@ import { threadStartResult, turnStartResult, } from "./run-attempt-test-harness.js"; +import { readCodexAppServerBinding } from "./session-binding.js"; + +type ReplyBackend = Parameters< + NonNullable["replyOperation"]>["attachBackend"] +>[0]; function flushDiagnosticEvents() { return waitForDiagnosticEventsDrained(); @@ -437,6 +444,402 @@ describe("runCodexAppServerAttempt hooks and model diagnostics", () => { expect(settled).toBe(true); }); + it("freezes recovered timeout success locally before agent_end", async () => { + let releaseAgentEnd: () => void = () => undefined; + const agentEndSettled = new Promise((resolve) => { + releaseAgentEnd = resolve; + }); + const agentEnd = vi.fn(() => agentEndSettled); + initializeGlobalHookRunner( + createMockPluginRegistry([{ hookName: "agent_end", handler: agentEnd }]), + ); + const onRunAgentEvent = vi.fn(); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.onAgentEvent = onRunAgentEvent; + params.timeoutMs = 200; + const attachBackend = vi.fn(); + const detachBackend = vi.fn(); + const freezeAbort = vi.fn(); + params.replyOperation = { + attachBackend, + detachBackend, + freezeAbort, + } as unknown as NonNullable; + const harness = createStartedThreadHarness(); + const run = runCodexAppServerAttempt(params, { + turnAssistantCompletionIdleTimeoutMs: 5, + turnTerminalIdleTimeoutMs: 500, + }); + + await harness.waitForMethod("turn/start"); + await harness.notify({ + method: "item/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + id: "msg-final-1", + type: "agentMessage", + text: "Done.", + status: "completed", + }, + }, + }); + await vi.waitFor(() => expect(agentEnd).toHaveBeenCalledTimes(1), fastWait); + + const [replyBackend] = mockCall(attachBackend, "reply backend") as [ + { isAbortable?: () => boolean }, + ]; + expect(replyBackend.isAbortable?.()).toBe(false); + expect(abortAgentHarnessRun("session-1")).toBe(false); + expect(resolveActiveEmbeddedRunSessionId("agent:main:session-1")).toBe("session-1"); + releaseAgentEnd(); + + await expect(run).resolves.toMatchObject({ + aborted: false, + timedOut: false, + promptError: null, + }); + const [agentEndPayload] = mockCall(agentEnd, "agent_end") as [{ success?: boolean }, unknown]; + expect(agentEndPayload.success).toBe(true); + expect(freezeAbort).not.toHaveBeenCalled(); + const terminalLifecycleEvents = onRunAgentEvent.mock.calls + .map(([event]) => event) + .filter( + (event) => + event.stream === "lifecycle" && + (event.data.phase === "end" || event.data.phase === "error"), + ); + expect(terminalLifecycleEvents).toHaveLength(1); + expect(terminalLifecycleEvents[0]?.data).toMatchObject({ phase: "end" }); + expect(terminalLifecycleEvents[0]?.data.aborted).toBeUndefined(); + expect(detachBackend).toHaveBeenCalledWith(replyBackend); + expect(resolveActiveEmbeddedRunSessionId("agent:main:session-1")).toBeUndefined(); + }); + + it("freezes recovered client-close success locally before agent_end", async () => { + let releaseAgentEnd: () => void = () => undefined; + const agentEndSettled = new Promise((resolve) => { + releaseAgentEnd = resolve; + }); + const agentEnd = vi.fn(() => agentEndSettled); + initializeGlobalHookRunner( + createMockPluginRegistry([{ hookName: "agent_end", handler: agentEnd }]), + ); + const onAttemptAbort = vi.fn(); + let replyBackend: Pick | undefined; + const params = createParams( + path.join(tempDir, "recovered-client-close.jsonl"), + path.join(tempDir, "recovered-client-close-workspace"), + ); + params.onAttemptAbort = onAttemptAbort; + params.replyOperation = { + attachBackend: (backend: ReplyBackend) => { + replyBackend = backend; + }, + detachBackend: vi.fn(), + freezeAbort: vi.fn(), + } as unknown as NonNullable; + const harness = createStartedThreadHarness(); + const run = runCodexAppServerAttempt(params, { turnTerminalIdleTimeoutMs: 60_000 }); + + await harness.waitForMethod("turn/start"); + await harness.notify({ + method: "item/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + id: "msg-final-1", + type: "agentMessage", + text: "Done before restart.", + status: "completed", + }, + }, + }); + harness.close(); + await vi.waitFor(() => expect(agentEnd).toHaveBeenCalledTimes(1), fastWait); + + expect(replyBackend?.isAbortable?.()).toBe(false); + expect(abortAgentHarnessRun("session-1")).toBe(false); + expect(onAttemptAbort).not.toHaveBeenCalled(); + + releaseAgentEnd(); + await expect(run).resolves.toMatchObject({ + aborted: false, + timedOut: false, + promptError: null, + assistantTexts: ["Done before restart."], + }); + const [agentEndPayload] = mockCall(agentEnd, "agent_end") as [{ success?: boolean }, unknown]; + expect(agentEndPayload.success).toBe(true); + }); + + it("keeps a successful memory preflight cancellable for the main turn", async () => { + let releaseAgentEnd: () => void = () => undefined; + const agentEndSettled = new Promise((resolve) => { + releaseAgentEnd = resolve; + }); + const agentEnd = vi.fn(() => agentEndSettled); + initializeGlobalHookRunner( + createMockPluginRegistry([{ hookName: "agent_end", handler: agentEnd }]), + ); + const onAttemptAbort = vi.fn(); + let replyBackend: Pick | undefined; + const params = createParams( + path.join(tempDir, "memory-preflight.jsonl"), + path.join(tempDir, "memory-preflight-workspace"), + ); + params.trigger = "memory"; + params.memoryFlushWritePath = "memory/notes.md"; + params.onAttemptAbort = onAttemptAbort; + const freezeAbort = vi.fn(); + params.replyOperation = { + attachBackend: (backend: ReplyBackend) => { + replyBackend = backend; + }, + detachBackend: vi.fn(), + freezeAbort, + } as unknown as NonNullable; + const harness = createStartedThreadHarness(); + const run = runCodexAppServerAttempt(params); + + await harness.waitForMethod("turn/start"); + await harness.completeTurn({ threadId: "thread-1", turnId: "turn-1" }); + await vi.waitFor(() => expect(agentEnd).toHaveBeenCalledTimes(1), fastWait); + + expect(replyBackend?.isAbortable?.()).toBe(true); + replyBackend?.cancel("user_abort"); + expect(onAttemptAbort).toHaveBeenCalledTimes(1); + + releaseAgentEnd(); + await expect(run).resolves.toMatchObject({ + aborted: false, + promptError: null, + }); + expect(freezeAbort).not.toHaveBeenCalled(); + }); + + it("keeps replay-safe client-close recovery cancellable during agent_end", async () => { + let releaseAgentEnd: () => void = () => undefined; + const agentEndSettled = new Promise((resolve) => { + releaseAgentEnd = resolve; + }); + const agentEnd = vi.fn(() => agentEndSettled); + initializeGlobalHookRunner( + createMockPluginRegistry([{ hookName: "agent_end", handler: agentEnd }]), + ); + const onAttemptAbort = vi.fn(); + let replyBackend: + | { + isAbortable?: () => boolean; + cancel: (reason: "restart" | "superseded" | "user_abort") => void; + } + | undefined; + const params = createParams( + path.join(tempDir, "replay-safe-client-close.jsonl"), + path.join(tempDir, "replay-safe-client-close-workspace"), + ); + params.onAttemptAbort = onAttemptAbort; + const freezeAbort = vi.fn(); + params.replyOperation = { + attachBackend: (backend: ReplyBackend) => { + replyBackend = backend; + }, + detachBackend: vi.fn(), + freezeAbort, + } as unknown as NonNullable; + const harness = createStartedThreadHarness(); + const run = runCodexAppServerAttempt(params, { turnTerminalIdleTimeoutMs: 60_000 }); + + await harness.waitForMethod("turn/start"); + harness.close(); + await vi.waitFor(() => expect(agentEnd).toHaveBeenCalledTimes(1), fastWait); + + expect(replyBackend?.isAbortable?.()).toBe(true); + replyBackend?.cancel("user_abort"); + expect(onAttemptAbort).toHaveBeenCalledTimes(1); + + releaseAgentEnd(); + await expect(run).resolves.toMatchObject({ + aborted: false, + promptError: "codex app-server client closed before turn completed", + codexAppServerFailure: { + kind: "client_closed_before_turn_completed", + replaySafe: true, + }, + }); + expect(freezeAbort).not.toHaveBeenCalled(); + }); + + it.each([ + { + label: "failed", + status: "failed", + error: { message: "codex exploded" }, + expectedPromptError: "codex exploded", + expectedClassification: undefined, + }, + { + label: "interrupted", + status: "interrupted", + error: undefined, + expectedPromptError: null, + expectedClassification: "empty", + }, + { + label: "empty completed", + status: "completed", + error: undefined, + expectedPromptError: null, + expectedClassification: "empty", + }, + ] as const)( + "keeps ordinary $label turns cancellable until the orchestrator settles", + async ({ label, status, error, expectedPromptError, expectedClassification }) => { + let releaseAgentEnd: () => void = () => undefined; + const agentEndSettled = new Promise((resolve) => { + releaseAgentEnd = resolve; + }); + const agentEnd = vi.fn(() => agentEndSettled); + initializeGlobalHookRunner( + createMockPluginRegistry([{ hookName: "agent_end", handler: agentEnd }]), + ); + const onAttemptAbort = vi.fn(); + let replyBackend: Pick | undefined; + const params = createParams( + path.join(tempDir, `ordinary-${label}-turn.jsonl`), + path.join(tempDir, `ordinary-${label}-turn-workspace`), + ); + params.onAttemptAbort = onAttemptAbort; + const freezeAbort = vi.fn(); + params.replyOperation = { + attachBackend: (backend: ReplyBackend) => { + replyBackend = backend; + }, + detachBackend: vi.fn(), + freezeAbort, + } as unknown as NonNullable; + const harness = createStartedThreadHarness(); + const run = runCodexAppServerAttempt(params); + + await harness.waitForMethod("turn/start"); + await harness.notify({ + method: "turn/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + turn: { + id: "turn-1", + status, + ...(error ? { error } : {}), + }, + }, + }); + await vi.waitFor(() => expect(agentEnd).toHaveBeenCalledTimes(1), fastWait); + + expect(replyBackend?.isAbortable?.()).toBe(true); + replyBackend?.cancel("user_abort"); + expect(onAttemptAbort).toHaveBeenCalledTimes(1); + + releaseAgentEnd(); + const result = await run; + expect(result).toMatchObject({ + aborted: false, + promptError: expectedPromptError, + }); + expect(result.agentHarnessResultClassification).toBe(expectedClassification); + expect(freezeAbort).not.toHaveBeenCalled(); + }, + ); + + it("keeps websocket client-close failure cancellable until the orchestrator settles", async () => { + let releaseAgentEnd: () => void = () => undefined; + const agentEndSettled = new Promise((resolve) => { + releaseAgentEnd = resolve; + }); + const agentEnd = vi.fn(() => agentEndSettled); + initializeGlobalHookRunner( + createMockPluginRegistry([{ hookName: "agent_end", handler: agentEnd }]), + ); + const onAttemptAbort = vi.fn(); + let replyBackend: + | { + isAbortable?: () => boolean; + cancel: (reason: "restart" | "superseded" | "user_abort") => void; + } + | undefined; + const params = createParams( + path.join(tempDir, "websocket-client-close.jsonl"), + path.join(tempDir, "websocket-client-close-workspace"), + ); + params.onAttemptAbort = onAttemptAbort; + params.replyOperation = { + attachBackend: (backend: ReplyBackend) => { + replyBackend = backend; + }, + detachBackend: vi.fn(), + freezeAbort: vi.fn(), + } as unknown as NonNullable; + const harness = createStartedThreadHarness(); + const run = runCodexAppServerAttempt(params, { + pluginConfig: { + appServer: { + transport: "websocket", + url: "ws://127.0.0.1:39175", + }, + }, + turnTerminalIdleTimeoutMs: 60_000, + }); + + await harness.waitForMethod("turn/start"); + harness.close(); + await vi.waitFor(() => expect(agentEnd).toHaveBeenCalledTimes(1), fastWait); + + expect(replyBackend?.isAbortable?.()).toBe(true); + replyBackend?.cancel("user_abort"); + expect(onAttemptAbort).toHaveBeenCalledTimes(1); + + releaseAgentEnd(); + await expect(run).resolves.toMatchObject({ + aborted: false, + promptError: "codex app-server client closed before turn completed", + codexAppServerFailure: { + transport: "websocket", + }, + }); + }); + + it("clears a stale binding when completed-turn coverage persistence fails", async () => { + const sessionFile = path.join(tempDir, "binding-coverage-failure.jsonl"); + const workspaceDir = path.join(tempDir, "binding-coverage-workspace"); + const harness = createStartedThreadHarness(); + const run = runCodexAppServerAttempt(createParams(sessionFile, workspaceDir)); + await harness.waitForMethod("turn/start"); + + const bindingPath = `${sessionFile}.codex-app-server.json`; + const originalWriteFile = fs.writeFile.bind(fs); + let rejectedCoverageWrite = false; + const writeFileSpy = vi.spyOn(fs, "writeFile").mockImplementation(async (...args) => { + if (!rejectedCoverageWrite && typeof args[0] === "string" && args[0] === bindingPath) { + rejectedCoverageWrite = true; + throw new Error("simulated binding coverage write failure"); + } + return originalWriteFile(...args); + }); + try { + await harness.completeTurn({ threadId: "thread-1", turnId: "turn-1" }); + await expect(run).resolves.toMatchObject({ promptError: null, aborted: false }); + expect(rejectedCoverageWrite).toBe(true); + await expect(readCodexAppServerBinding(sessionFile)).resolves.toBeUndefined(); + } finally { + writeFileSpy.mockRestore(); + } + }); + it("does not wait for agent_end hooks before resolving channel-backed codex turns", async () => { let releaseAgentEnd: () => void = () => undefined; const agentEndSettled = new Promise((resolve) => { diff --git a/extensions/codex/src/app-server/run-attempt.ts b/extensions/codex/src/app-server/run-attempt.ts index 209301dd7ed8..fd33dd388934 100644 --- a/extensions/codex/src/app-server/run-attempt.ts +++ b/extensions/codex/src/app-server/run-attempt.ts @@ -84,6 +84,8 @@ import { reportCodexExecutionNotification, } from "./attempt-notification-state.js"; import { + describeNotificationActivity, + isAssistantCompletionReleaseNotification, isCodexNotificationOutsideActiveRun, isCurrentApprovalTurnRequestParams, isCurrentThreadOptionalTurnRequestParams, @@ -277,6 +279,22 @@ import { resolveCodexWebSearchPlan } from "./web-search.js"; const CODEX_NATIVE_HOOK_RELAY_RENEW_INTERVAL_MS = 60_000; const CODEX_APP_SERVER_PROJECTED_CHARS_PER_TOKEN = 4; const CODEX_APP_SERVER_ACTIVE_NATIVE_TURN_WAIT_TIMEOUT_MS = 30_000; + +function shouldKeepCodexSharedAbortOpen(params: { + trigger: EmbeddedRunAttemptParams["trigger"]; + result: EmbeddedRunAttemptResult; + attemptSucceeded: boolean; + explicitCancellationObserved: boolean; +}): boolean { + if (params.explicitCancellationObserved || params.result.aborted || params.result.externalAbort) { + return false; + } + // Memory attempts are preparatory. Failed attempts can still enter runner + // retries or model fallback. The reply orchestrator owns the shared terminal + // freeze after those paths settle. + return params.trigger === "memory" || !params.attemptSucceeded; +} + const ensuredCodexWorkspaceDirs = new Set(); function withCodexAppServerFastModeServiceTier( @@ -576,8 +594,34 @@ export async function runCodexAppServerAttempt( preDynamicStartupStages.mark("native-hook-relay"); const runAbortController = new AbortController(); + // AbortController preserves its first reason, so retain explicit cancellation + // that can arrive after the timeout abort while cleanup is still draining. + let explicitCancellationObserved = false; + let explicitCancellationReason: unknown; + let terminalOutcomeFrozen = false; + let sharedAbortAllowedAfterTerminalOutcome = false; + let attemptAbortNotified = false; + const notifyAttemptAbort = () => { + if (attemptAbortNotified) { + return; + } + attemptAbortNotified = true; + params.onAttemptAbort?.(); + }; + const abortExplicitly = (reason: unknown) => { + if (terminalOutcomeFrozen) { + if (sharedAbortAllowedAfterTerminalOutcome) { + notifyAttemptAbort(); + } + return; + } + notifyAttemptAbort(); + explicitCancellationObserved = true; + explicitCancellationReason ??= reason; + runAbortController.abort(reason); + }; const abortFromUpstream = () => { - runAbortController.abort(params.abortSignal?.reason ?? "upstream_abort"); + abortExplicitly(params.abortSignal?.reason ?? "upstream_abort"); }; if (params.abortSignal?.aborted) { abortFromUpstream(); @@ -1629,6 +1673,10 @@ export async function runCodexAppServerAttempt( const pendingOpenClawDynamicToolCompletionIds = new Set(); const activeTurnItemIds = new Set(); const activeCompletionBlockerItemIds = new Set(); + const activeFinalizationHookRunIds = new Set(); + const finalizationHookBatchStatuses = new Map(); + let unsettledFinalizationHookCount = 0; + let rejectedFinalizationHookAssistant: { itemId?: string } | undefined; let turnCrossedToolHandoff = false; let pendingTerminalDynamicToolRelease: | { @@ -1679,6 +1727,9 @@ export async function runCodexAppServerAttempt( getActiveAppServerTurnRequests: () => activeAppServerTurnRequests, getActiveTurnItemCount: () => activeTurnItemIds.size, getActiveCompletionBlockerItemCount: () => activeCompletionBlockerItemIds.size, + getActiveFinalizationHookCount: () => unsettledFinalizationHookCount, + canReleaseAssistantCompletionIdle: () => + projectorRef.current?.hasLatestTerminalAssistantCandidateText() === true, turnCompletionIdleTimeoutMs, turnAssistantCompletionIdleTimeoutMs, turnAttemptIdleTimeoutMs, @@ -1960,6 +2011,22 @@ export async function runCodexAppServerAttempt( onReportExecutionNotification: reportExecutionNotification, }); turnCrossedToolHandoff = notificationState.turnCrossedToolHandoff; + const finalizationHookNotification = readCodexFinalizationHookNotification( + notification, + thread.threadId, + turnId, + ); + if (finalizationHookNotification?.phase === "started") { + // Codex emits every start in one Stop/SubagentStop batch before it runs + // any handler, then emits completions. An empty active set starts a batch. + if (activeFinalizationHookRunIds.size === 0) { + finalizationHookBatchStatuses.clear(); + } + activeFinalizationHookRunIds.add(finalizationHookNotification.runId); + // The receive-time disarm may precede an earlier queued assistant + // completion. Repeat it in notification order after that completion. + turnWatches.disarmAssistantCompletionIdleWatch(); + } // Determine terminal-turn status before invoking the projector so a throw // inside projector.handleNotification still releases the session lane. // See openclaw/openclaw#67996. @@ -1969,6 +2036,39 @@ export async function runCodexAppServerAttempt( try { await waitForCodexNotificationDispatchTurn(); await projector.handleNotification(notification); + const projectedAssistantCompletionCanRelease = + isAssistantCompletionReleaseNotification(notification, turnCrossedToolHandoff) || + (notificationState.isCurrentTurnNotification && + turnCrossedToolHandoff && + notification.method === "rawResponseItem/completed" && + projector.canReleaseLatestTerminalAssistantAfterToolHandoff()); + if (notificationState.isCurrentTurnNotification && projectedAssistantCompletionCanRelease) { + const completedAssistantItemId = projector.getLatestTerminalAssistantCandidate()?.itemId; + if ( + rejectedFinalizationHookAssistant !== undefined && + completedAssistantItemId !== undefined && + completedAssistantItemId !== rejectedFinalizationHookAssistant.itemId + ) { + rejectedFinalizationHookAssistant = undefined; + } else if (rejectedFinalizationHookAssistant !== undefined) { + // A delayed raw echo still belongs to the rejected assistant. + turnWatches.disarmAssistantCompletionIdleWatch(); + } else { + const canArmProjectedAssistantCompletion = + activeFinalizationHookRunIds.size === 0 && + !terminalTurnNotificationQueued && + activeAppServerTurnRequests === 0 && + activeTurnItemIds.size === 0 && + activeCompletionBlockerItemIds.size === 0 && + pendingOpenClawDynamicToolCompletionIds.size === 0 && + projector.hasLatestTerminalAssistantCandidateText(); + if (canArmProjectedAssistantCompletion) { + // Receive-time arming can expire while an earlier queued projection + // is still running. Restart the idle window from committed output. + turnWatches.armAssistantCompletionIdleWatch(describeNotificationActivity(notification)); + } + } + } if ( notificationState.isCurrentTurnNotification && activeTurnItemIds.size === 0 && @@ -1982,6 +2082,41 @@ export async function runCodexAppServerAttempt( error, }); } finally { + if (finalizationHookNotification?.phase === "completed") { + unsettledFinalizationHookCount = Math.max(0, unsettledFinalizationHookCount - 1); + activeFinalizationHookRunIds.delete(finalizationHookNotification.runId); + finalizationHookBatchStatuses.set( + finalizationHookNotification.runId, + finalizationHookNotification.status, + ); + if (activeFinalizationHookRunIds.size === 0) { + const statuses = new Set(finalizationHookBatchStatuses.values()); + const aggregateBlocked = statuses.has("blocked") && !statuses.has("stopped"); + if (aggregateBlocked) { + const itemId = projector.getLatestTerminalAssistantCandidate()?.itemId; + rejectedFinalizationHookAssistant = itemId ? { itemId } : {}; + turnWatches.disarmAssistantCompletionIdleWatch(); + } else { + rejectedFinalizationHookAssistant = undefined; + } + } + const canRearmAssistantCompletionWatch = + activeFinalizationHookRunIds.size === 0 && + rejectedFinalizationHookAssistant === undefined && + !terminalTurnNotificationQueued && + activeAppServerTurnRequests === 0 && + activeTurnItemIds.size === 0 && + activeCompletionBlockerItemIds.size === 0 && + pendingOpenClawDynamicToolCompletionIds.size === 0 && + projector.hasLatestTerminalAssistantCandidateText(); + if (canRearmAssistantCompletionWatch) { + turnWatches.armAssistantCompletionIdleWatch({ + lastNotificationMethod: notification.method, + hookRunId: finalizationHookNotification.runId, + hookStatus: finalizationHookNotification.status, + }); + } + } if (notificationState.isTurnTerminal) { if (notificationState.isTurnAbortMarker) { projector.markAborted(); @@ -2105,6 +2240,17 @@ export async function runCodexAppServerAttempt( } } if (notificationMatchesActiveTurn) { + const finalizationHookNotification = readCodexFinalizationHookNotification( + notification, + thread.threadId, + turnId, + ); + if (finalizationHookNotification?.phase === "started") { + unsettledFinalizationHookCount += 1; + // Codex runs finalization hooks after completing the assistant item. + // Suspend recovery until those hooks accept or replace that answer. + turnWatches.disarmAssistantCompletionIdleWatch(); + } // If Codex app-server exposes raw response deltas, treat them as activity // only when scoped to this turn or attributable to a single lease. turnWatches.noteNotificationReceived( @@ -2126,6 +2272,15 @@ export async function runCodexAppServerAttempt( ); return notificationQueue; }; + const drainNotificationQueue = async (): Promise => { + while (true) { + const queued = notificationQueue; + await queued; + if (queued === notificationQueue) { + return; + } + } + }; const nativeSubagentCodexHome = appServer.start.transport === "stdio" @@ -2883,16 +3038,26 @@ export async function runCodexAppServerAttempt( steeringQueueRef.current = activeSteeringQueue; const handle = { kind: "embedded" as const, + runId: params.runId, queueMessage: async (text: string, optionsLocal?: CodexSteeringQueueOptions) => activeSteeringQueue.queue(text, optionsLocal), isStreaming: () => !completed && !runAbortController.signal.aborted, isStopped: () => completed || timedOut || runAbortController.signal.aborted, + isAbortable: () => !terminalOutcomeFrozen || sharedAbortAllowedAfterTerminalOutcome, isCompacting: () => projectorRef.current?.isCompacting() ?? false, sourceReplyDeliveryMode: params.sourceReplyDeliveryMode, - cancel: () => runAbortController.abort("cancelled"), - abort: () => runAbortController.abort("aborted"), + cancel: () => abortExplicitly("cancelled"), + abort: () => abortExplicitly("aborted"), }; + params.replyOperation?.attachBackend(handle); setActiveEmbeddedRun(params.sessionId, handle, params.sessionKey, params.sessionFile); + const freezeRunTerminalOutcome = () => { + if (terminalOutcomeFrozen) { + return; + } + terminalOutcomeFrozen = true; + params.abortSignal?.removeEventListener("abort", abortFromUpstream); + }; const notifyUserMessagePersisted = createCodexAppServerUserMessagePersistenceNotifier(params); void mirrorPromptAtTurnStartBestEffort({ params, @@ -2937,24 +3102,62 @@ export async function runCodexAppServerAttempt( // projected, for example while persisting raw image-generation media. Wait // for already-queued projection work so the final result includes artifacts // from the notification that triggered the idle watchdog. - await notificationQueue; - const result = activeProjector.buildResult(toolBridge.telemetry, { yieldDetected }); - const finalAborted = - result.aborted || (runAbortController.signal.aborted && !clientClosedAbort); - const canUseCompletedAssistantTextAfterClientClose = + await drainNotificationQueue(); + const hasQuiescentCompletedAssistant = activeProjector.hasCompletedTerminalAssistantText() && activeAppServerTurnRequests === 0 && activeTurnItemIds.size === 0 && - pendingOpenClawDynamicToolCompletionIds.size === 0; + activeCompletionBlockerItemIds.size === 0 && + pendingOpenClawDynamicToolCompletionIds.size === 0 && + activeFinalizationHookRunIds.size === 0 && + unsettledFinalizationHookCount === 0 && + rejectedFinalizationHookAssistant === undefined; + const hasRecoverableCompletedAssistant = + !turnWatches.isCompletionIdleWatchPinnedByTerminalError() && + turnWatches.isAssistantCompletionIdleWatchArmed() && + hasQuiescentCompletedAssistant; + const recoveredTurnWatchTimeout = + turnCompletionIdleTimedOut && + !explicitCancellationObserved && + !terminalTurnNotificationQueued && + hasRecoverableCompletedAssistant && + activeProjector.recoverCompletedTerminalAssistantAfterTurnWatchTimeout(); + if (recoveredTurnWatchTimeout) { + embeddedAgentLog.warn( + "codex app-server recovered completed assistant output after missing turn completion", + { + threadId: thread.threadId, + turnId: activeTurnId, + timeoutKind: turnWatchTimeoutKind, + idleMs: turnWatchTimeoutIdleMs, + timeoutMs: turnWatchTimeoutMs, + }, + ); + trajectoryRecorder?.recordEvent("turn.watch_timeout_recovered", { + threadId: thread.threadId, + turnId: activeTurnId, + timeoutKind: turnWatchTimeoutKind, + idleMs: turnWatchTimeoutIdleMs, + timeoutMs: turnWatchTimeoutMs, + }); + } + const result = activeProjector.buildResult(toolBridge.telemetry, { yieldDetected }); + const effectiveTimedOut = timedOut && !recoveredTurnWatchTimeout; + const effectiveTurnCompletionIdleTimedOut = + turnCompletionIdleTimedOut && !recoveredTurnWatchTimeout; + const isFinalAborted = () => + result.aborted || + explicitCancellationObserved || + (runAbortController.signal.aborted && !clientClosedAbort && !recoveredTurnWatchTimeout); const clientClosedPromptErrorForFinal = - clientClosedPromptError && canUseCompletedAssistantTextAfterClientClose + clientClosedPromptError && hasRecoverableCompletedAssistant ? undefined : clientClosedPromptError; let finalPromptError = clientClosedPromptErrorForFinal ?? - (turnCompletionIdleTimedOut + (effectiveTurnCompletionIdleTimedOut ? turnCompletionIdleTimeoutMessage - : timedOut + : effectiveTimedOut ? "codex app-server attempt timed out" : result.promptError); const finalPromptErrorMessage = @@ -3002,10 +3205,10 @@ export async function runCodexAppServerAttempt( finalPromptError = refreshedUsageLimitPromptError; } const finalPromptErrorSource = - timedOut || clientClosedPromptErrorForFinal ? "prompt" : result.promptErrorSource; + effectiveTimedOut || clientClosedPromptErrorForFinal ? "prompt" : result.promptErrorSource; const codexAppServerFailureKind = clientClosedPromptErrorForFinal ? "client_closed_before_turn_completed" - : turnCompletionIdleTimedOut + : effectiveTurnCompletionIdleTimedOut ? "turn_completion_idle_timeout" : undefined; const codexAppServerReplayBlockedReason = codexAppServerFailureKind @@ -3013,7 +3216,7 @@ export async function runCodexAppServerAttempt( : undefined; const promptTimeoutOutcome = buildCodexAppServerPromptTimeoutOutcome({ result, - turnCompletionIdleTimedOut, + turnCompletionIdleTimedOut: effectiveTurnCompletionIdleTimedOut, turnWatchTimeoutKind, }); const codexAppServerFailureDiagnostics = @@ -3026,13 +3229,56 @@ export async function runCodexAppServerAttempt( details: turnWatchTimeoutDetails, }) : undefined; + const codexAppServerFailure = codexAppServerFailureKind + ? ({ + kind: codexAppServerFailureKind, + ...(codexAppServerFailureKind === "turn_completion_idle_timeout" && turnWatchTimeoutKind + ? { turnWatchTimeoutKind } + : {}), + transport: appServer.start.transport, + threadId: thread.threadId, + turnId: activeTurnId, + replaySafe: codexAppServerReplayBlockedReason === undefined, + ...(codexAppServerReplayBlockedReason + ? { replayBlockedReason: codexAppServerReplayBlockedReason } + : {}), + ...(codexAppServerFailureDiagnostics + ? { diagnostics: codexAppServerFailureDiagnostics } + : {}), + } satisfies NonNullable) + : undefined; + const finalAborted = isFinalAborted(); + const completedTurnStatus = activeProjector.getCompletedTurnStatus(); + const completedWithoutTerminalNotification = + completed && + !terminalTurnNotificationQueued && + !timedOut && + clientClosedPromptErrorForFinal === undefined; + const attemptSucceeded = + !finalAborted && + !effectiveTimedOut && + (finalPromptError === null || finalPromptError === undefined) && + result.agentHarnessResultClassification === undefined && + (completedTurnStatus === "completed" || + recoveredTurnWatchTimeout || + completedWithoutTerminalNotification); + sharedAbortAllowedAfterTerminalOutcome = shouldKeepCodexSharedAbortOpen({ + trigger: params.trigger, + result, + attemptSucceeded, + explicitCancellationObserved, + }); + // Terminal diagnostics, transcript mirroring, hooks, and lifecycle events + // must all observe one immutable attempt outcome. Failed attempts still + // allow the shared reply operation to cancel retries or model fallback. + freezeRunTerminalOutcome(); const modelCallFailureKind = classifyCodexModelCallFailureKind({ error: finalPromptError, - timedOut, - turnCompletionIdleTimedOut, - runAborted: runAbortController.signal.aborted, - abortReason: runAbortController.signal.reason, + timedOut: effectiveTimedOut, + turnCompletionIdleTimedOut: effectiveTurnCompletionIdleTimedOut, + runAborted: finalAborted, + abortReason: explicitCancellationReason ?? runAbortController.signal.reason, clientClosedAbort, formatError: formatErrorMessage, }) ?? (finalAborted ? "aborted" : undefined); @@ -3048,23 +3294,6 @@ export async function runCodexAppServerAttempt( } else { codexModelCallDiagnostics.emitCompleted(result); } - recordCodexTrajectoryCompletion(trajectoryRecorder, { - attempt: params, - result, - threadId: thread.threadId, - turnId: activeTurnId, - timedOut, - yieldDetected, - }); - trajectoryRecorder?.recordEvent("session.ended", { - status: finalPromptError ? "error" : finalAborted || timedOut ? "interrupted" : "success", - threadId: thread.threadId, - turnId: activeTurnId, - timedOut, - yieldDetected, - promptError: normalizeCodexTrajectoryError(finalPromptError), - }); - markTrajectoryEndRecorded(); await mirrorTranscriptBestEffort({ params, agentId: sessionAgentId, @@ -3075,29 +3304,6 @@ export async function runCodexAppServerAttempt( threadId: thread.threadId, turnId: activeTurnId, }); - const terminalAssistantText = collectTerminalAssistantText(result); - if ( - terminalAssistantText && - (!assistantStreamEventEmitted || assistantStreamNeedsTerminalSnapshot) && - !finalAborted && - !finalPromptError - ) { - void emitCodexAppServerEvent(params, { - stream: "assistant", - data: { text: terminalAssistantText }, - }); - } - if (finalPromptError) { - emitLifecycleTerminal({ - phase: "error", - error: formatErrorMessage(finalPromptError), - }); - } else { - emitLifecycleTerminal({ - phase: "end", - ...(finalAborted ? { aborted: true } : {}), - }); - } if (activeContextEngine) { const activeContextEnginePluginIdLocal = resolveContextEngineOwnerPluginId(activeContextEngine); @@ -3172,49 +3378,95 @@ export async function runCodexAppServerAttempt( ctx: hookContext, hookRunner, }); - const completedTurnStatus = activeProjector.getCompletedTurnStatus(); shouldDelayNativeHookRelayUnregister = completedTurnStatus === "completed" && - !timedOut && + !effectiveTimedOut && !runAbortController.signal.aborted && !finalAborted && !finalPromptError; if (shouldDelayNativeHookRelayUnregister) { - await markCodexAppServerBindingCoveredThroughTurn({ - sessionFile: params.sessionFile, - threadId: thread.threadId, - authProfileStore: params.authProfileStore, - agentDir: params.agentDir, - config: params.config, + try { + await markCodexAppServerBindingCoveredThroughTurn({ + sessionFile: params.sessionFile, + threadId: thread.threadId, + authProfileStore: params.authProfileStore, + agentDir: params.agentDir, + config: params.config, + }); + } catch (error) { + const clearedStaleBinding = await clearCodexAppServerBindingForThread( + params.sessionFile, + thread.threadId, + { + authProfileStore: params.authProfileStore, + agentDir: params.agentDir, + config: params.config, + }, + ); + if (!clearedStaleBinding) { + throw error; + } + embeddedAgentLog.warn( + "codex app-server binding coverage update failed after completed turn; cleared stale binding", + { + threadId: thread.threadId, + turnId: activeTurnId, + error, + }, + ); + } + } + recordCodexTrajectoryCompletion(trajectoryRecorder, { + attempt: params, + result, + threadId: thread.threadId, + turnId: activeTurnId, + timedOut: effectiveTimedOut, + yieldDetected, + }); + trajectoryRecorder?.recordEvent("session.ended", { + status: finalPromptError + ? "error" + : finalAborted || effectiveTimedOut + ? "interrupted" + : "success", + threadId: thread.threadId, + turnId: activeTurnId, + timedOut: effectiveTimedOut, + yieldDetected, + promptError: normalizeCodexTrajectoryError(finalPromptError), + }); + markTrajectoryEndRecorded(); + const terminalAssistantText = collectTerminalAssistantText(result); + if ( + terminalAssistantText && + (!assistantStreamEventEmitted || assistantStreamNeedsTerminalSnapshot) && + !finalAborted && + !finalPromptError + ) { + void emitCodexAppServerEvent(params, { + stream: "assistant", + data: { text: terminalAssistantText }, + }); + } + if (finalPromptError) { + emitLifecycleTerminal({ + phase: "error", + error: formatErrorMessage(finalPromptError), + }); + } else { + emitLifecycleTerminal({ + phase: "end", + ...(finalAborted ? { aborted: true } : {}), }); } return { ...result, - timedOut, + timedOut: effectiveTimedOut, aborted: finalAborted, promptError: finalPromptError, promptErrorSource: finalPromptErrorSource, - ...(codexAppServerFailureKind - ? { - codexAppServerFailure: { - kind: codexAppServerFailureKind, - ...(codexAppServerFailureKind === "turn_completion_idle_timeout" && - turnWatchTimeoutKind - ? { turnWatchTimeoutKind } - : {}), - transport: appServer.start.transport, - threadId: thread.threadId, - turnId: activeTurnId, - replaySafe: codexAppServerReplayBlockedReason === undefined, - ...(codexAppServerReplayBlockedReason - ? { replayBlockedReason: codexAppServerReplayBlockedReason } - : {}), - ...(codexAppServerFailureDiagnostics - ? { diagnostics: codexAppServerFailureDiagnostics } - : {}), - }, - } - : {}), + ...(codexAppServerFailure ? { codexAppServerFailure } : {}), ...(promptTimeoutOutcome ? { promptTimeoutOutcome } : {}), systemPromptReport, }; @@ -3280,8 +3532,9 @@ export async function runCodexAppServerAttempt( } await releaseSandboxExecEnvironment(); runAbortController.signal.removeEventListener("abort", abortListener); - params.abortSignal?.removeEventListener("abort", abortFromUpstream); steeringQueueRef.current?.cancel(); + freezeRunTerminalOutcome(); + params.replyOperation?.detachBackend(handle); clearActiveEmbeddedRun(params.sessionId, handle, params.sessionKey, params.sessionFile); } } @@ -3409,6 +3662,40 @@ function isCodexThreadTurnCompletedNotification( return !turnIds || (turnId !== undefined && turnIds.has(turnId)); } +function readCodexFinalizationHookNotification( + notification: CodexServerNotification, + threadId: string, + turnId: string, +): + | { phase: "started"; runId: string } + | { phase: "completed"; runId: string; status: string | undefined } + | undefined { + if (notification.method !== "hook/started" && notification.method !== "hook/completed") { + return undefined; + } + const params = isJsonObject(notification.params) ? notification.params : undefined; + const run = params && isJsonObject(params.run) ? params.run : undefined; + // Codex selects exactly one of Stop or SubagentStop from the turn's session + // source, so these event names share aggregation state but cannot coexist. + if ( + params?.threadId !== threadId || + params.turnId !== turnId || + (run?.eventName !== "stop" && run?.eventName !== "subagentStop") || + typeof run.id !== "string" || + !run.id + ) { + return undefined; + } + if (notification.method === "hook/started") { + return { phase: "started", runId: run.id }; + } + return { + phase: "completed", + runId: run.id, + status: typeof run.status === "string" ? run.status : undefined, + }; +} + function joinPresentSections(...sections: Array): string { return sections.filter((section): section is string => Boolean(section?.trim())).join("\n\n"); } diff --git a/extensions/codex/src/app-server/run-attempt.turn-watches.test.ts b/extensions/codex/src/app-server/run-attempt.turn-watches.test.ts index 3bc843919adb..6fa64a3e48d0 100644 --- a/extensions/codex/src/app-server/run-attempt.turn-watches.test.ts +++ b/extensions/codex/src/app-server/run-attempt.turn-watches.test.ts @@ -10,6 +10,7 @@ import { onInternalDiagnosticEvent, type DiagnosticEventPayload, } from "openclaw/plugin-sdk/diagnostic-runtime"; +import * as mediaStore from "openclaw/plugin-sdk/media-store"; import { describe, expect, it, vi } from "vitest"; import { createCodexAttemptTurnWatchController } from "./attempt-turn-watches.js"; import * as authBridge from "./auth-bridge.js"; @@ -64,6 +65,104 @@ function writeCodexAppServerBinding(...args: Parameters { it("reschedules the attempt watch when notification progress shortens its timeout", async () => { const onTimeout = vi.fn(); @@ -77,6 +176,8 @@ describe("createCodexAttemptTurnWatchController", () => { getActiveAppServerTurnRequests: () => 0, getActiveTurnItemCount: () => 0, getActiveCompletionBlockerItemCount: () => 0, + getActiveFinalizationHookCount: () => 0, + canReleaseAssistantCompletionIdle: () => true, turnCompletionIdleTimeoutMs: 500, turnAssistantCompletionIdleTimeoutMs: 500, turnAttemptIdleTimeoutMs: 200, @@ -272,7 +373,6 @@ describe("runCodexAppServerAttempt turn watches", () => { }, }, }); - const result = await run; expect(result.timedOut).toBe(true); @@ -285,6 +385,46 @@ describe("runCodexAppServerAttempt turn watches", () => { }); }); + it("keeps partial assistant deltas on the timeout path", async () => { + const harness = createStartedThreadHarness(); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.timeoutMs = 200; + + const run = runCodexAppServerAttempt(params, { + pluginConfig: { appServer: { turnCompletionIdleTimeoutMs: 5 } }, + turnAssistantCompletionIdleTimeoutMs: 1_000, + turnTerminalIdleTimeoutMs: 60_000, + }); + await harness.waitForMethod("turn/start"); + await harness.notify({ + method: "item/agentMessage/delta", + params: { + threadId: "thread-1", + turnId: "turn-1", + itemId: "msg-partial-1", + delta: "Still writing", + }, + }); + + const result = await run; + + expect(result).toMatchObject({ + aborted: true, + timedOut: true, + promptError: "codex app-server turn idle timed out waiting for turn/completed", + assistantTexts: ["Still writing"], + codexAppServerFailure: { + kind: "turn_completion_idle_timeout", + turnWatchTimeoutKind: "progress", + replaySafe: false, + replayBlockedReason: "assistant_output", + }, + }); + }); + it("preserves raw image-generation media when Codex never sends turn completion", async () => { const harness = createStartedThreadHarness(); const params = createParams( @@ -419,43 +559,187 @@ describe("runCodexAppServerAttempt turn watches", () => { expect(result.promptTimeoutOutcome).toBeUndefined(); }); - it("does not use completion timeout outcome for non-completion timeout with assistant output", async () => { - const harness = createStartedThreadHarness(); - const params = createParams( - path.join(tempDir, "session.jsonl"), - path.join(tempDir, "workspace"), - ); - params.timeoutMs = 100; + it("recovers completed assistant output from a non-completion timeout", async () => { + const warn = vi.spyOn(embeddedAgentLog, "warn").mockImplementation(() => undefined); + const { params, result } = await runTurnWatchTimeoutScenario([ + completedCommand("cmd-1", "touch done.txt"), + completedAssistant("msg-1", "Finished."), + ]); - const run = runCodexAppServerAttempt(params, { - turnCompletionIdleTimeoutMs: 500, - turnAssistantCompletionIdleTimeoutMs: 1_000, - turnTerminalIdleTimeoutMs: 500, + expect(result).toMatchObject({ + aborted: false, + timedOut: false, + promptError: null, + assistantTexts: ["Finished."], }); - await harness.waitForMethod("turn/start"); - await harness.notify({ - method: "item/completed", - params: { + expect(result.itemLifecycle.completedCount).toBe(2); + expect(result.codexAppServerFailure).toBeUndefined(); + expect(result.promptTimeoutOutcome).toBeUndefined(); + expect(warn).toHaveBeenCalledWith( + "codex app-server recovered completed assistant output after missing turn completion", + expect.objectContaining({ + timeoutKind: "progress", threadId: "thread-1", turnId: "turn-1", - item: { - id: "msg-1", - type: "agentMessage", - text: "Finished.", - status: "completed", + }), + ); + expect(await readCodexAppServerBinding(params.sessionFile)).toBeUndefined(); + }); + + it("preserves a rewritten completed assistant after its id-less raw echo", async () => { + const { result } = await runTurnWatchTimeoutScenario([ + completedAssistant("msg-1", "Contributor-rewritten answer."), + { + method: "rawResponseItem/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + type: "message", + role: "assistant", + content: [{ type: "output_text", text: "Original model answer." }], + }, }, }, + ]); + + expect(result).toMatchObject({ + aborted: false, + timedOut: false, + promptError: null, + assistantTexts: ["Contributor-rewritten answer."], }); + expect(result.codexAppServerFailure).toBeUndefined(); + }); - const result = await run; + it("uses an id-less raw echo when the typed completion is blank", async () => { + const { result } = await runTurnWatchTimeoutScenario([ + completedAssistant("msg-1", " "), + { + method: "rawResponseItem/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + type: "message", + role: "assistant", + content: [{ type: "output_text", text: "Raw fallback answer." }], + }, + }, + }, + ]); + expect(result).toMatchObject({ + aborted: false, + timedOut: false, + promptError: null, + assistantTexts: ["Raw fallback answer."], + }); + expect(result.codexAppServerFailure).toBeUndefined(); + }); + + it("does not recover an assistant followed by a raw tool call", async () => { + const { result } = await runTurnWatchTimeoutScenario([ + completedAssistant("msg-1", "I will run a tool."), + { + method: "rawResponseItem/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + type: "custom_tool_call", + id: "tool-raw-1", + name: "shell", + input: '{"command":"echo pending"}', + }, + }, + }, + ]); + + expect(result.aborted).toBe(true); expect(result.timedOut).toBe(true); - expect(result.itemLifecycle.completedCount).toBe(1); - expect(result.toolMetas).toEqual([]); - expect(result.codexAppServerFailure?.turnWatchTimeoutKind).not.toBe("completion"); - expect(result.codexAppServerFailure?.replaySafe).toBe(false); - expect(result.codexAppServerFailure?.replayBlockedReason).toBe("assistant_output"); - expect(result.codexAppServerFailure?.diagnostics).toBeUndefined(); + expect(result.promptError).toBe( + "codex app-server turn idle timed out waiting for turn/completed", + ); + expect(result.assistantTexts).toEqual(["I will run a tool."]); + expect(result.codexAppServerFailure?.turnWatchTimeoutKind).toBe("progress"); + }); + + it.each([ + { + name: "while an item is active", + notifications: [ + startedCommand("cmd-1", "touch unfinished.txt"), + completedAssistant("msg-1", "Finished."), + ], + assistantText: "Finished.", + activeCount: 1, + completedCount: 1, + timeoutKind: "progress", + replayBlockedReason: "potential_side_effect", + }, + { + name: "when an earlier item finishes later", + notifications: [ + startedCommand("cmd-1", "touch finishes-later.txt"), + completedAssistant("msg-1", "Too early."), + completedCommand("cmd-1", "touch finishes-later.txt"), + ], + assistantText: "Too early.", + activeCount: 0, + completedCount: 2, + timeoutKind: "progress", + replayBlockedReason: "potential_side_effect", + }, + { + name: "after a later completed item", + notifications: [ + completedAssistant("msg-1", "Earlier answer."), + startedCommand("cmd-1", "touch done-later.txt"), + completedCommand("cmd-1", "touch done-later.txt"), + ], + assistantText: "Earlier answer.", + activeCount: 0, + completedCount: 2, + timeoutKind: "terminal", + replayBlockedReason: "potential_side_effect", + }, + { + name: "after a newer empty assistant completion", + notifications: [completedAssistant("msg-1", "Earlier answer."), completedAssistant("msg-2")], + assistantText: "Earlier answer.", + activeCount: 0, + completedCount: 2, + timeoutKind: "progress", + replayBlockedReason: "assistant_output", + }, + ] satisfies Array<{ + name: string; + notifications: CodexServerNotification[]; + assistantText: string; + activeCount: number; + completedCount: number; + timeoutKind: "progress" | "terminal"; + replayBlockedReason: "assistant_output" | "potential_side_effect"; + }>)("keeps completed assistant output on the timeout path $name", async (scenario) => { + const { result } = await runTurnWatchTimeoutScenario(scenario.notifications); + + expect(result).toMatchObject({ + aborted: true, + timedOut: true, + promptError: "codex app-server turn idle timed out waiting for turn/completed", + assistantTexts: [scenario.assistantText], + itemLifecycle: { + activeCount: scenario.activeCount, + completedCount: scenario.completedCount, + }, + codexAppServerFailure: { + kind: "turn_completion_idle_timeout", + turnWatchTimeoutKind: scenario.timeoutKind, + replaySafe: false, + replayBlockedReason: scenario.replayBlockedReason, + }, + }); expect(result.promptTimeoutOutcome).toBeUndefined(); }); @@ -3770,6 +4054,825 @@ describe("runCodexAppServerAttempt turn watches", () => { ); }); + it.each(["stop", "subagentStop"] as const)( + "waits for an active %s hook before recovering a completed assistant", + async (eventName) => { + const harness = createStartedThreadHarness(); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.timeoutMs = 200; + + const run = runCodexAppServerAttempt(params, { + turnAssistantCompletionIdleTimeoutMs: 10, + turnTerminalIdleTimeoutMs: 500, + }); + await harness.waitForMethod("turn/start"); + await harness.notify(completedAssistant("msg-final-1", "Done.")); + await harness.notify(finalizationHookNotification("hook/started", "running", eventName)); + await new Promise((resolve) => { + setTimeout(resolve, 25); + }); + + expect(harness.requests).not.toContainEqual( + expect.objectContaining({ method: "turn/interrupt" }), + ); + + await harness.notify(finalizationHookNotification("hook/completed", "completed", eventName)); + const result = await run; + + expect(result).toMatchObject({ + aborted: false, + timedOut: false, + promptError: null, + assistantTexts: ["Done."], + }); + expect(harness.requests).toContainEqual( + expect.objectContaining({ method: "turn/interrupt" }), + ); + }, + ); + + it("keeps recovery suspended when a finalization hook queues behind an assistant", async () => { + let releaseProjection!: () => void; + let markProjectionStarted!: () => void; + const projectionGate = new Promise((resolve) => { + releaseProjection = resolve; + }); + const projectionStarted = new Promise((resolve) => { + markProjectionStarted = resolve; + }); + vi.spyOn(mediaStore, "saveMediaBuffer").mockImplementation(async () => { + markProjectionStarted(); + await projectionGate; + throw new Error("expected projection gate"); + }); + const harness = createStartedThreadHarness(); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.timeoutMs = 200; + + const run = runCodexAppServerAttempt(params, { + turnAssistantCompletionIdleTimeoutMs: 10, + turnTerminalIdleTimeoutMs: 500, + }); + await harness.waitForMethod("turn/start"); + const pendingProjection = harness.notify({ + method: "rawResponseItem/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + id: "ig_raw_1", + type: "image_generation_call", + status: "generating", + result: tinyPngBase64, + }, + }, + }); + await projectionStarted; + const pendingAssistant = harness.notify(completedAssistant("msg-final-1", "Done.")); + const pendingHook = harness.notify(finalizationHookNotification("hook/started", "running")); + releaseProjection(); + await Promise.all([pendingProjection, pendingAssistant, pendingHook]); + await new Promise((resolve) => { + setTimeout(resolve, 25); + }); + + expect(harness.requests).not.toContainEqual( + expect.objectContaining({ method: "turn/interrupt" }), + ); + + await harness.notify(finalizationHookNotification("hook/completed", "completed")); + await expect(run).resolves.toMatchObject({ + aborted: false, + timedOut: false, + promptError: null, + assistantTexts: ["Done."], + }); + }); + + it("rearms recovery after queued assistant projection outlives the receive-time watch", async () => { + let releaseProjection!: () => void; + let markProjectionStarted!: () => void; + const projectionGate = new Promise((resolve) => { + releaseProjection = resolve; + }); + const projectionStarted = new Promise((resolve) => { + markProjectionStarted = resolve; + }); + vi.spyOn(mediaStore, "saveMediaBuffer").mockImplementation(async () => { + markProjectionStarted(); + await projectionGate; + throw new Error("expected projection gate"); + }); + const harness = createStartedThreadHarness(); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.timeoutMs = 200; + + const run = runCodexAppServerAttempt(params, { + turnAssistantCompletionIdleTimeoutMs: 5, + turnTerminalIdleTimeoutMs: 500, + }); + await harness.waitForMethod("turn/start"); + const pendingProjection = harness.notify({ + method: "rawResponseItem/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + id: "ig_raw_1", + type: "image_generation_call", + status: "generating", + result: tinyPngBase64, + }, + }, + }); + await projectionStarted; + const pendingAssistant = harness.notify(completedAssistant("msg-final-1", "Done.")); + await new Promise((resolve) => { + setTimeout(resolve, 15); + }); + expect(harness.requests).not.toContainEqual( + expect.objectContaining({ method: "turn/interrupt" }), + ); + + releaseProjection(); + await Promise.all([pendingProjection, pendingAssistant]); + await expect(run).resolves.toMatchObject({ + aborted: false, + timedOut: false, + promptError: null, + assistantTexts: ["Done."], + }); + expect(harness.requests).toContainEqual(expect.objectContaining({ method: "turn/interrupt" })); + }); + + it("does not rearm recovery for an assistant superseded during finalization", async () => { + const harness = createStartedThreadHarness(); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.timeoutMs = 200; + + let settled = false; + const run = runCodexAppServerAttempt(params, { + turnAssistantCompletionIdleTimeoutMs: 10, + turnTerminalIdleTimeoutMs: 500, + }).finally(() => { + settled = true; + }); + await harness.waitForMethod("turn/start"); + await harness.notify(completedAssistant("msg-final-1", "Stale answer.")); + await harness.notify(finalizationHookNotification("hook/started", "running")); + await harness.notify(completedCommand("cmd-later-1", "echo later")); + await harness.notify(finalizationHookNotification("hook/completed", "completed")); + await new Promise((resolve) => { + setTimeout(resolve, 25); + }); + + expect(settled).toBe(false); + expect(harness.requests).not.toContainEqual( + expect.objectContaining({ method: "turn/interrupt" }), + ); + + harness.close(); + await expect(run).resolves.toMatchObject({ + aborted: false, + timedOut: false, + promptError: "codex app-server client closed before turn completed", + assistantTexts: ["Stale answer."], + codexAppServerFailure: { + replaySafe: false, + replayBlockedReason: "potential_side_effect", + }, + }); + }); + + it("does not recover a delayed raw echo after later work starts", async () => { + const harness = createStartedThreadHarness(); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.timeoutMs = 200; + + let settled = false; + const run = runCodexAppServerAttempt(params, { + turnAssistantCompletionIdleTimeoutMs: 10, + turnTerminalIdleTimeoutMs: 500, + }).finally(() => { + settled = true; + }); + await harness.waitForMethod("turn/start"); + await harness.notify(completedAssistant("msg-final-1", "Stale answer.")); + await harness.notify(startedCommand("cmd-later-1", "echo later")); + await harness.notify({ + method: "rawResponseItem/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + type: "message", + role: "assistant", + content: [{ type: "output_text", text: "Stale answer." }], + }, + }, + }); + await harness.notify(completedCommand("cmd-later-1", "echo later")); + await new Promise((resolve) => { + setTimeout(resolve, 25); + }); + + expect(settled).toBe(false); + expect(harness.requests).not.toContainEqual( + expect.objectContaining({ method: "turn/interrupt" }), + ); + + harness.close(); + await expect(run).resolves.toMatchObject({ + aborted: false, + timedOut: false, + promptError: "codex app-server client closed before turn completed", + assistantTexts: ["Stale answer."], + codexAppServerFailure: { + replaySafe: false, + replayBlockedReason: "potential_side_effect", + }, + }); + }); + + it("does not revive a superseded assistant from an unpaired raw echo", async () => { + const harness = createStartedThreadHarness(); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.timeoutMs = 200; + + let settled = false; + const run = runCodexAppServerAttempt(params, { + turnAssistantCompletionIdleTimeoutMs: 10, + turnTerminalIdleTimeoutMs: 500, + }).finally(() => { + settled = true; + }); + await harness.waitForMethod("turn/start"); + await harness.notify(completedAssistant("msg-final-1", "Stale answer.")); + await harness.notify({ + method: "item/agentMessage/delta", + params: { + threadId: "thread-1", + turnId: "turn-1", + itemId: "msg-partial-2", + delta: "Newer partial answer.", + }, + }); + await harness.notify({ + method: "rawResponseItem/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + id: "raw-final-1", + type: "message", + role: "assistant", + content: [{ type: "output_text", text: "Stale answer." }], + }, + }, + }); + await new Promise((resolve) => { + setTimeout(resolve, 25); + }); + + expect(settled).toBe(false); + expect(harness.requests).not.toContainEqual( + expect.objectContaining({ method: "turn/interrupt" }), + ); + + harness.close(); + await expect(run).resolves.toMatchObject({ + aborted: false, + timedOut: false, + promptError: "codex app-server client closed before turn completed", + assistantTexts: ["Newer partial answer."], + codexAppServerFailure: { + replaySafe: false, + replayBlockedReason: "assistant_output", + }, + }); + }); + + it("treats a differently identified raw assistant as newer output", async () => { + const harness = createStartedThreadHarness(); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.timeoutMs = 200; + + const run = runCodexAppServerAttempt(params, { + turnAssistantCompletionIdleTimeoutMs: 10, + turnTerminalIdleTimeoutMs: 500, + }); + await harness.waitForMethod("turn/start"); + await harness.notify(completedAssistant("msg-final-1", "Earlier answer.")); + await harness.notify({ + method: "rawResponseItem/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + id: "raw-final-2", + type: "message", + role: "assistant", + content: [{ type: "output_text", text: "Newer raw answer." }], + }, + }, + }); + + await expect(run).resolves.toMatchObject({ + aborted: false, + timedOut: false, + promptError: null, + assistantTexts: ["Newer raw answer."], + }); + }); + + it("treats an id-less raw assistant after later completed work as newer output", async () => { + const harness = createStartedThreadHarness(); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.timeoutMs = 200; + + const run = runCodexAppServerAttempt(params, { + turnAssistantCompletionIdleTimeoutMs: 10, + turnTerminalIdleTimeoutMs: 500, + }); + await harness.waitForMethod("turn/start"); + await harness.notify(completedAssistant("msg-pre-tool", "I will check.")); + await harness.notify(startedCommand("cmd-later-1", "echo checked")); + await harness.notify(completedCommand("cmd-later-1", "echo checked")); + await harness.notify({ + method: "rawResponseItem/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + type: "message", + role: "assistant", + content: [{ type: "output_text", text: "Checked successfully." }], + }, + }, + }); + + await expect(run).resolves.toMatchObject({ + aborted: false, + timedOut: false, + promptError: null, + assistantTexts: ["Checked successfully."], + }); + }); + + it("keeps recovery valid through raw output for an earlier active item", async () => { + const harness = createStartedThreadHarness(); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.timeoutMs = 200; + + const run = runCodexAppServerAttempt(params, { + turnAssistantCompletionIdleTimeoutMs: 10, + turnTerminalIdleTimeoutMs: 500, + }); + await harness.waitForMethod("turn/start"); + await harness.notify({ + method: "item/started", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + id: "ig_1", + type: "imageGeneration", + status: "inProgress", + }, + }, + }); + await harness.notify(completedAssistant("msg-final-1", "Done.")); + await harness.notify({ + method: "item/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + id: "ig_1", + type: "imageGeneration", + status: "completed", + }, + }, + }); + await harness.notify({ + method: "rawResponseItem/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + id: "ig_1", + type: "image_generation_call", + status: "completed", + result: tinyPngBase64, + }, + }, + }); + + await expect(run).resolves.toMatchObject({ + aborted: false, + timedOut: false, + promptError: null, + assistantTexts: ["Done."], + }); + }); + + it("does not rearm recovery after a newer assistant starts streaming", async () => { + const harness = createStartedThreadHarness(); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.timeoutMs = 200; + + let settled = false; + const run = runCodexAppServerAttempt(params, { + turnAssistantCompletionIdleTimeoutMs: 10, + turnTerminalIdleTimeoutMs: 500, + }).finally(() => { + settled = true; + }); + await harness.waitForMethod("turn/start"); + await harness.notify(completedAssistant("msg-final-1", "Stale answer.")); + await harness.notify(finalizationHookNotification("hook/started", "running")); + await harness.notify({ + method: "item/agentMessage/delta", + params: { + threadId: "thread-1", + turnId: "turn-1", + itemId: "msg-partial-2", + delta: "Newer partial answer.", + }, + }); + await harness.notify(finalizationHookNotification("hook/completed", "completed")); + await new Promise((resolve) => { + setTimeout(resolve, 25); + }); + + expect(settled).toBe(false); + expect(harness.requests).not.toContainEqual( + expect.objectContaining({ method: "turn/interrupt" }), + ); + + harness.close(); + await expect(run).resolves.toMatchObject({ + aborted: false, + timedOut: false, + promptError: "codex app-server client closed before turn completed", + assistantTexts: ["Newer partial answer."], + codexAppServerFailure: { + replaySafe: false, + replayBlockedReason: "assistant_output", + }, + }); + }); + + it.each([ + { + name: "blocked then stopped", + completions: [ + { runId: "stop-hook-blocked", status: "blocked" as const }, + { runId: "stop-hook-stopped", status: "stopped" as const }, + ], + }, + { + name: "stopped then blocked", + completions: [ + { runId: "stop-hook-stopped", status: "stopped" as const }, + { runId: "stop-hook-blocked", status: "blocked" as const }, + ], + }, + ])("honors a stopped finalization override when hooks complete $name", async (scenario) => { + const harness = createStartedThreadHarness(); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.timeoutMs = 200; + + const run = runCodexAppServerAttempt(params, { + turnAssistantCompletionIdleTimeoutMs: 10, + turnTerminalIdleTimeoutMs: 500, + }); + await harness.waitForMethod("turn/start"); + await harness.notify(completedAssistant("msg-final-1", "Accepted answer.")); + await harness.notify( + finalizationHookNotification("hook/started", "running", "stop", "stop-hook-blocked"), + ); + await harness.notify( + finalizationHookNotification("hook/started", "running", "stop", "stop-hook-stopped"), + ); + for (const completion of scenario.completions) { + await harness.notify( + finalizationHookNotification("hook/completed", completion.status, "stop", completion.runId), + ); + } + + await expect(run).resolves.toMatchObject({ + aborted: false, + timedOut: false, + promptError: null, + assistantTexts: ["Accepted answer."], + }); + }); + + it("recovers an accepted raw-only assistant after finalization hooks", async () => { + const harness = createStartedThreadHarness(); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.timeoutMs = 200; + + const run = runCodexAppServerAttempt(params, { + turnAssistantCompletionIdleTimeoutMs: 10, + turnTerminalIdleTimeoutMs: 500, + }); + await harness.waitForMethod("turn/start"); + await harness.notify({ + method: "rawResponseItem/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + id: "raw-final-1", + type: "message", + role: "assistant", + content: [{ type: "output_text", text: "Accepted raw answer." }], + }, + }, + }); + await harness.notify(finalizationHookNotification("hook/started", "running")); + await harness.notify(finalizationHookNotification("hook/completed", "completed")); + + await expect(run).resolves.toMatchObject({ + aborted: false, + timedOut: false, + promptError: null, + assistantTexts: ["Accepted raw answer."], + }); + }); + + it("recovers a revised raw-only assistant after finalization rejection", async () => { + const harness = createStartedThreadHarness(); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.timeoutMs = 200; + + const run = runCodexAppServerAttempt(params, { + turnAssistantCompletionIdleTimeoutMs: 10, + turnTerminalIdleTimeoutMs: 500, + }); + await harness.waitForMethod("turn/start"); + await harness.notify({ + method: "rawResponseItem/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + id: "raw-final-1", + type: "message", + role: "assistant", + content: [{ type: "output_text", text: "Rejected raw answer." }], + }, + }, + }); + await harness.notify(finalizationHookNotification("hook/started", "running")); + await harness.notify(finalizationHookNotification("hook/completed", "blocked")); + await harness.notify({ + method: "rawResponseItem/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + id: "raw-final-2", + type: "message", + role: "assistant", + content: [{ type: "output_text", text: "Revised raw answer." }], + }, + }, + }); + + await expect(run).resolves.toMatchObject({ + aborted: false, + timedOut: false, + promptError: null, + assistantTexts: ["Revised raw answer."], + }); + }); + + it("does not recover an assistant rejected by a Stop hook", async () => { + const harness = createStartedThreadHarness(); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.timeoutMs = 200; + + const run = runCodexAppServerAttempt(params, { + turnAssistantCompletionIdleTimeoutMs: 10, + turnTerminalIdleTimeoutMs: 500, + }); + await harness.waitForMethod("turn/start"); + await harness.notify(completedAssistant("msg-final-1", " ")); + await harness.notify(finalizationHookNotification("hook/started", "running")); + await harness.notify(finalizationHookNotification("hook/completed", "blocked")); + await harness.notify({ + method: "rawResponseItem/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + type: "message", + role: "assistant", + content: [{ type: "output_text", text: "Rejected answer." }], + }, + }, + }); + await new Promise((resolve) => { + setTimeout(resolve, 25); + }); + + expect(harness.requests).not.toContainEqual( + expect.objectContaining({ method: "turn/interrupt" }), + ); + + await harness.notify(completedAssistant("msg-final-2", "Revised answer.")); + const result = await run; + + expect(result).toMatchObject({ + aborted: false, + timedOut: false, + promptError: null, + assistantTexts: ["Revised answer."], + }); + }); + + it("keeps upstream cancellation aborted when it races with timeout recovery", async () => { + let releaseProjection!: () => void; + let markProjectionStarted!: () => void; + const projectionGate = new Promise((resolve) => { + releaseProjection = resolve; + }); + const projectionStarted = new Promise((resolve) => { + markProjectionStarted = resolve; + }); + vi.spyOn(mediaStore, "saveMediaBuffer").mockImplementation(async () => { + markProjectionStarted(); + await projectionGate; + throw new Error("expected projection gate"); + }); + const harness = createStartedThreadHarness(); + const abortController = new AbortController(); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.abortSignal = abortController.signal; + params.timeoutMs = 200; + + const run = runCodexAppServerAttempt(params, { + turnAssistantCompletionIdleTimeoutMs: 5, + turnTerminalIdleTimeoutMs: 500, + }); + await harness.waitForMethod("turn/start"); + await harness.notify(completedAssistant("msg-final-1", "Done.")); + const pendingProjection = harness.notify({ + method: "rawResponseItem/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + id: "ig_raw_1", + type: "image_generation_call", + status: "generating", + result: tinyPngBase64, + }, + }, + }); + await projectionStarted; + await new Promise((resolve) => { + setTimeout(resolve, 15); + }); + expect(harness.requests).not.toContainEqual( + expect.objectContaining({ method: "turn/interrupt" }), + ); + + abortController.abort("user_cancelled"); + releaseProjection(); + await pendingProjection; + + await expect(run).resolves.toMatchObject({ + aborted: true, + timedOut: false, + promptError: null, + }); + }); + + it("does not recover a completed reply when a queued native abort marker follows it", async () => { + let releaseProjection!: () => void; + let markProjectionStarted!: () => void; + const projectionGate = new Promise((resolve) => { + releaseProjection = resolve; + }); + const projectionStarted = new Promise((resolve) => { + markProjectionStarted = resolve; + }); + vi.spyOn(mediaStore, "saveMediaBuffer").mockImplementation(async () => { + markProjectionStarted(); + await projectionGate; + throw new Error("expected projection gate"); + }); + const harness = createStartedThreadHarness(); + const params = createParams( + path.join(tempDir, "session.jsonl"), + path.join(tempDir, "workspace"), + ); + params.timeoutMs = 200; + + const run = runCodexAppServerAttempt(params, { + turnAssistantCompletionIdleTimeoutMs: 5, + turnTerminalIdleTimeoutMs: 500, + }); + await harness.waitForMethod("turn/start"); + await harness.notify(completedAssistant("msg-final-1", "Done.")); + const pendingProjection = harness.notify({ + method: "rawResponseItem/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + id: "ig_raw_1", + type: "image_generation_call", + status: "generating", + result: tinyPngBase64, + }, + }, + }); + await projectionStarted; + await new Promise((resolve) => { + setTimeout(resolve, 15); + }); + expect(harness.requests).not.toContainEqual( + expect.objectContaining({ method: "turn/interrupt" }), + ); + const pendingAbort = harness.notify({ + method: "rawResponseItem/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + id: "abort-marker-1", + type: "message", + role: "user", + content: [ + { + type: "input_text", + text: "\nThe user interrupted the previous turn on purpose. Any running unified exec processes may still be running in the background. If any tools/commands were aborted, they may have partially executed.\n", + }, + ], + }, + }, + }); + + releaseProjection(); + await Promise.all([pendingProjection, pendingAbort]); + + await expect(run).resolves.toMatchObject({ + aborted: true, + timedOut: false, + promptError: null, + }); + }); + it("keeps waiting when a current-turn item is still active", async () => { let notify: (notification: CodexServerNotification) => Promise = async () => undefined; const request = vi.fn(async (method: string) => { @@ -4199,6 +5302,77 @@ describe("runCodexAppServerAttempt turn watches", () => { }); }); + it.each([ + { + name: "after a newer empty completion", + notifications: [ + completedAssistant("msg-1", "Earlier complete reply."), + completedAssistant("msg-2"), + ], + assistantText: "Earlier complete reply.", + replayBlockedReason: "assistant_output", + }, + { + name: "after a later completed item", + notifications: [ + completedAssistant("msg-1", "Earlier complete reply."), + startedCommand("cmd-1", "touch later.txt"), + completedCommand("cmd-1", "touch later.txt"), + ], + assistantText: "Earlier complete reply.", + replayBlockedReason: "potential_side_effect", + }, + { + name: "when an earlier item finishes later", + notifications: [ + startedCommand("cmd-1", "touch finishes-later.txt"), + completedAssistant("msg-1", "Too early."), + completedCommand("cmd-1", "touch finishes-later.txt"), + ], + assistantText: "Too early.", + replayBlockedReason: "potential_side_effect", + }, + { + name: "after a later raw tool call", + notifications: [ + completedAssistant("msg-1", "I will run a tool."), + { + method: "rawResponseItem/completed", + params: { + threadId: "thread-1", + turnId: "turn-1", + item: { + type: "custom_tool_call", + id: "tool-raw-1", + name: "shell", + input: '{"command":"echo pending"}', + }, + }, + }, + ], + assistantText: "I will run a tool.", + replayBlockedReason: "assistant_output", + }, + ] satisfies Array<{ + name: string; + notifications: CodexServerNotification[]; + assistantText: string; + replayBlockedReason: "assistant_output" | "potential_side_effect"; + }>)("keeps completed assistant output as a client-close failure $name", async (scenario) => { + const result = await runClientCloseScenario(scenario.notifications); + + expect(result.promptError).toBe("codex app-server client closed before turn completed"); + expect(result.assistantTexts).toEqual([scenario.assistantText]); + expect(result.codexAppServerFailure).toEqual({ + kind: "client_closed_before_turn_completed", + transport: "stdio", + threadId: "thread-1", + turnId: "turn-1", + replaySafe: false, + replayBlockedReason: scenario.replayBlockedReason, + }); + }); + it("keeps completed assistant output as a client-close failure while another item is active", async () => { const harness = createStartedThreadHarness(); const run = runCodexAppServerAttempt( diff --git a/src/agents/agent-command.live-model-switch.test.ts b/src/agents/agent-command.live-model-switch.test.ts index b1b0fa3f4fd5..2c80c48a1877 100644 --- a/src/agents/agent-command.live-model-switch.test.ts +++ b/src/agents/agent-command.live-model-switch.test.ts @@ -3,7 +3,10 @@ import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vite import type { SessionEntry } from "../config/sessions.js"; import { INTERNAL_RUNTIME_CONTEXT_BEGIN, INTERNAL_RUNTIME_CONTEXT_END } from "./internal-events.js"; import { LiveSessionModelSwitchError } from "./live-model-switch-error.js"; -import { createAgentRunRestartAbortError } from "./run-termination.js"; +import { + createAgentRunDirectAbortError, + createAgentRunRestartAbortError, +} from "./run-termination.js"; const state = vi.hoisted(() => ({ defaultRuntimeConfig: { @@ -2853,6 +2856,32 @@ describe("agentCommand – LiveSessionModelSwitchError retry", () => { ).toBe(true); }); + it("marks direct active-run cancellation aborted without a caller signal", async () => { + state.runWithModelFallbackMock.mockRejectedValueOnce(createAgentRunDirectAbortError()); + + await expect( + agentCommand({ + message: "hello", + to: "+1234567890", + }), + ).rejects.toThrow("agent run aborted"); + + expect( + state.emitAgentEventMock.mock.calls.some(([event]) => { + const candidate = event as { + stream?: string; + data?: { phase?: string; aborted?: boolean; stopReason?: string }; + }; + return ( + candidate.stream === "lifecycle" && + candidate.data?.phase === "error" && + candidate.data.aborted === true && + candidate.data.stopReason === "aborted" + ); + }), + ).toBe(true); + }); + it("propagates authProfileId from the switch error to the retried session entry", async () => { let capturedAuthProfileProvider: string | undefined; setupModelSwitchRetry({ diff --git a/src/agents/agent-command.ts b/src/agents/agent-command.ts index 17df23239aa6..63537cf4be6f 100644 --- a/src/agents/agent-command.ts +++ b/src/agents/agent-command.ts @@ -135,6 +135,7 @@ import { import { listOpenAIAuthProfileProvidersForAgentRuntime } from "./openai-routing.js"; import { resolveProviderIdForAuth } from "./provider-auth-aliases.js"; import { + isAgentRunDirectAbortReason, isAgentRunRestartAbortReason, resolveAgentRunAbortLifecycleFields, } from "./run-termination.js"; @@ -2127,6 +2128,9 @@ async function agentCommandInternal( continue; } if (!attemptLifecycleState.lifecycleEnded) { + const abortLifecycleFields = isAgentRunDirectAbortReason(err) + ? { aborted: true as const, stopReason: "aborted" as const } + : resolveAgentRunAbortLifecycleFields(opts.abortSignal); emitAgentEvent({ runId, lifecycleGeneration, @@ -2136,7 +2140,7 @@ async function agentCommandInternal( startedAt, endedAt: Date.now(), error: err instanceof Error ? err.message : "Agent run failed", - ...resolveAgentRunAbortLifecycleFields(opts.abortSignal), + ...abortLifecycleFields, }, }); } diff --git a/src/agents/embedded-agent-runner/run-state.ts b/src/agents/embedded-agent-runner/run-state.ts index aca4cfebd3d5..1f21b544db6f 100644 --- a/src/agents/embedded-agent-runner/run-state.ts +++ b/src/agents/embedded-agent-runner/run-state.ts @@ -18,9 +18,11 @@ import { resolveGlobalSingleton } from "../../shared/global-singleton.js"; */ export type EmbeddedAgentQueueHandle = { kind?: "embedded"; + runId?: string; queueMessage: (text: string, options?: EmbeddedAgentQueueMessageOptions) => Promise; isStreaming: () => boolean; isStopped?: () => boolean; + isAbortable?: () => boolean; isCompacting: () => boolean; supportsTranscriptCommitWait?: boolean; cancel?: (reason?: "user_abort" | "restart" | "superseded") => void; @@ -59,6 +61,8 @@ const EMBEDDED_RUN_STATE_KEY = Symbol.for("openclaw.embeddedRunState"); const embeddedRunState = resolveGlobalSingleton(EMBEDDED_RUN_STATE_KEY, () => ({ activeRuns: new Map(), + activeRunsByRunId: new Map(), + retainedAbortabilityRunIds: new Set(), snapshots: new Map(), sessionIdsByKey: new Map(), sessionIdsByFile: new Map(), @@ -71,6 +75,12 @@ const embeddedRunState = resolveGlobalSingleton(EMBEDDED_RUN_STATE_KEY, () => ({ export const ACTIVE_EMBEDDED_RUNS = embeddedRunState.activeRuns ?? (embeddedRunState.activeRuns = new Map()); +export const ACTIVE_EMBEDDED_RUNS_BY_RUN_ID = + embeddedRunState.activeRunsByRunId ?? + (embeddedRunState.activeRunsByRunId = new Map()); +export const RETAINED_EMBEDDED_RUN_ABORTABILITY_RUN_IDS = + embeddedRunState.retainedAbortabilityRunIds ?? + (embeddedRunState.retainedAbortabilityRunIds = new Set()); export const ACTIVE_EMBEDDED_RUN_SNAPSHOTS = embeddedRunState.snapshots ?? (embeddedRunState.snapshots = new Map()); diff --git a/src/agents/embedded-agent-runner/run.codex-app-server-recovery.test.ts b/src/agents/embedded-agent-runner/run.codex-app-server-recovery.test.ts index 2bab016422bb..becde855407f 100644 --- a/src/agents/embedded-agent-runner/run.codex-app-server-recovery.test.ts +++ b/src/agents/embedded-agent-runner/run.codex-app-server-recovery.test.ts @@ -1,5 +1,5 @@ // Coverage for replay-safe Codex app-server recovery retries. -import { beforeAll, beforeEach, describe, expect, it } from "vitest"; +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { makeModelFallbackCfg } from "../test-helpers/model-fallback-config-fixture.js"; import { makeAttemptResult } from "./run.overflow-compaction.fixture.js"; import { @@ -10,7 +10,8 @@ import { overflowBaseRunParams, resetRunOverflowCompactionHarnessMocks, } from "./run.overflow-compaction.harness.js"; -import type { EmbeddedRunAttemptResult } from "./run/types.js"; +import { hasCodexAppServerRecoveryRetryBudget } from "./run/codex-app-server-recovery.js"; +import type { EmbeddedRunAttemptParams, EmbeddedRunAttemptResult } from "./run/types.js"; let runEmbeddedAgent: typeof import("./run.js").runEmbeddedAgent; @@ -70,6 +71,18 @@ function successAttempt(): EmbeddedRunAttemptResult { }); } +function ordinaryPromptFailureAttempt(): EmbeddedRunAttemptResult { + return makeAttemptResult({ + assistantTexts: [], + promptError: new Error("codex exploded"), + promptErrorSource: "prompt", + }); +} + +function asAttemptParams(value: unknown): EmbeddedRunAttemptParams { + return value as EmbeddedRunAttemptParams; +} + describe("runEmbeddedAgent Codex app-server recovery", () => { beforeAll(async () => { ({ runEmbeddedAgent } = await loadRunOverflowCompactionHarness()); @@ -80,6 +93,23 @@ describe("runEmbeddedAgent Codex app-server recovery", () => { mockedClassifyFailoverReason.mockReturnValue(null); }); + it("does not advertise recovery after the outer run-loop budget is exhausted", () => { + expect( + hasCodexAppServerRecoveryRetryBudget({ + alreadyRetried: false, + runLoopIterations: 32, + maxRunLoopIterations: 32, + }), + ).toBe(false); + expect( + hasCodexAppServerRecoveryRetryBudget({ + alreadyRetried: false, + runLoopIterations: 31, + maxRunLoopIterations: 32, + }), + ).toBe(true); + }); + it("retries a replay-safe stdio client close once", async () => { mockedRunEmbeddedAttempt .mockResolvedValueOnce(codexClientClosedAttempt()) @@ -95,6 +125,132 @@ describe("runEmbeddedAgent Codex app-server recovery", () => { expect(mockedRunEmbeddedAttempt).toHaveBeenCalledTimes(2); }); + it("keeps shared abort ownership open through a replay-safe retry", async () => { + const freezeAbort = vi.fn(); + const replyOperation = { + freezeAbort, + } as unknown as NonNullable[0]["replyOperation"]>; + mockedRunEmbeddedAttempt + .mockImplementationOnce(async () => { + expect(freezeAbort).not.toHaveBeenCalled(); + return codexClientClosedAttempt(); + }) + .mockImplementationOnce(async () => { + expect(freezeAbort).not.toHaveBeenCalled(); + return successAttempt(); + }); + + await runEmbeddedAgent({ + ...overflowBaseRunParams, + provider: "codex", + model: "gpt-5.5", + runId: "run-codex-freeze-after-retry", + replyOperation, + }); + + expect(mockedRunEmbeddedAttempt).toHaveBeenCalledTimes(2); + expect(freezeAbort).not.toHaveBeenCalled(); + }); + + it("does not replay after cancellation during replay-safe finalization", async () => { + mockedRunEmbeddedAttempt.mockImplementationOnce(async (attemptParams) => { + asAttemptParams(attemptParams).onAttemptAbort?.(); + return codexClientClosedAttempt(); + }); + + await expect( + runEmbeddedAgent({ + ...overflowBaseRunParams, + provider: "codex", + model: "gpt-5.5", + runId: "run-codex-cancel-before-retry", + }), + ).rejects.toMatchObject({ + name: "AbortError", + message: "agent run aborted", + }); + + expect(mockedRunEmbeddedAttempt).toHaveBeenCalledTimes(1); + }); + + it("stops ordinary failure handling after cancellation during finalization", async () => { + mockedRunEmbeddedAttempt.mockImplementationOnce(async (attemptParams) => { + asAttemptParams(attemptParams).onAttemptAbort?.(); + return ordinaryPromptFailureAttempt(); + }); + + await expect( + runEmbeddedAgent({ + ...overflowBaseRunParams, + provider: "codex", + model: "gpt-5.5", + runId: "run-codex-cancel-ordinary-failure", + }), + ).rejects.toMatchObject({ + name: "AbortError", + message: "agent run aborted", + }); + + expect(mockedRunEmbeddedAttempt).toHaveBeenCalledTimes(1); + }); + + it("does not expose a cancelled replay-safe failure to outer model fallback", async () => { + const abortByUser = vi.fn(() => true); + const replyOperation = { + abortByUser, + } as unknown as NonNullable[0]["replyOperation"]>; + mockedRunEmbeddedAttempt.mockImplementationOnce(async (attemptParams) => { + asAttemptParams(attemptParams).onAttemptAbort?.(); + return codexClientClosedAttempt(); + }); + + await expect( + runEmbeddedAgent({ + ...overflowBaseRunParams, + provider: "codex", + model: "gpt-5.5", + runId: "run-codex-cancel-before-model-fallback", + isFinalFallbackAttempt: false, + replyOperation, + }), + ).rejects.toMatchObject({ + name: "AbortError", + message: "agent run aborted", + }); + + expect(mockedRunEmbeddedAttempt).toHaveBeenCalledTimes(1); + expect(abortByUser).toHaveBeenCalledTimes(1); + }); + + it("preserves upstream abort ownership during replay-safe finalization", async () => { + const controller = new AbortController(); + const upstreamAbort = new Error("upstream cancelled"); + const abortByUser = vi.fn(() => true); + const replyOperation = { + abortByUser, + } as unknown as NonNullable[0]["replyOperation"]>; + mockedRunEmbeddedAttempt.mockImplementationOnce(async (attemptParams) => { + controller.abort(upstreamAbort); + asAttemptParams(attemptParams).onAttemptAbort?.(); + return codexClientClosedAttempt(); + }); + + await expect( + runEmbeddedAgent({ + ...overflowBaseRunParams, + provider: "codex", + model: "gpt-5.5", + runId: "run-codex-upstream-cancel-before-model-fallback", + isFinalFallbackAttempt: false, + abortSignal: controller.signal, + replyOperation, + }), + ).rejects.toBe(upstreamAbort); + + expect(mockedRunEmbeddedAttempt).toHaveBeenCalledTimes(1); + expect(abortByUser).not.toHaveBeenCalled(); + }); + it("suppresses duplicate Codex prompt mirroring on retry", async () => { mockedRunEmbeddedAttempt .mockResolvedValueOnce(codexClientClosedAttempt()) @@ -244,6 +400,22 @@ describe("runEmbeddedAgent Codex app-server recovery", () => { expect(mockedMarkAuthProfileFailure).not.toHaveBeenCalled(); }); + it("keeps retry ownership open for an outer fallback after local recovery is exhausted", async () => { + mockedRunEmbeddedAttempt + .mockResolvedValueOnce(codexTurnCompletionIdleTimeoutAttempt()) + .mockResolvedValueOnce(codexTurnCompletionIdleTimeoutAttempt()); + + await runEmbeddedAgent({ + ...overflowBaseRunParams, + provider: "codex", + model: "gpt-5.5", + runId: "run-codex-turn-completion-outer-fallback", + isFinalFallbackAttempt: false, + }); + + expect(mockedRunEmbeddedAttempt).toHaveBeenCalledTimes(2); + }); + it("surfaces non-stdio turn/completed idle timeouts instead of throwing", async () => { mockedRunEmbeddedAttempt.mockResolvedValueOnce( codexTurnCompletionIdleTimeoutAttempt({ diff --git a/src/agents/embedded-agent-runner/run.ts b/src/agents/embedded-agent-runner/run.ts index 541bbc4ff52a..c0f671e60909 100644 --- a/src/agents/embedded-agent-runner/run.ts +++ b/src/agents/embedded-agent-runner/run.ts @@ -130,6 +130,7 @@ import { applyAgentRunSessionTargetIdentity, resolveAgentRunSessionTarget, } from "../run-session-target.js"; +import { createAgentRunDirectAbortError } from "../run-termination.js"; import { buildAgentRuntimeAuthPlan } from "../runtime-plan/auth.js"; import { buildAgentRuntimePlan } from "../runtime-plan/build.js"; import { ensureRuntimePluginsLoaded } from "../runtime-plugins.js"; @@ -180,7 +181,10 @@ import { forgetPromptBuildDrainCacheForRun } from "./run/attempt.prompt-helpers. import { createEmbeddedRunAuthController } from "./run/auth-controller.js"; import { resolveAuthProfileFailureReason } from "./run/auth-profile-failure-policy.js"; import { runEmbeddedAttemptWithBackend } from "./run/backend.js"; -import { resolveCodexAppServerRecoveryRetry } from "./run/codex-app-server-recovery.js"; +import { + hasCodexAppServerRecoveryRetryBudget, + resolveCodexAppServerRecoveryRetry, +} from "./run/codex-app-server-recovery.js"; import { createFailoverDecisionLogger } from "./run/failover-observation.js"; import { mergeRetryFailoverReason, resolveRunFailoverDecision } from "./run/failover-policy.js"; import { hasEmbeddedRunConfiguredModelFallbacks } from "./run/fallbacks.js"; @@ -2091,6 +2095,12 @@ async function runEmbeddedAgentInternal( ); timeoutReleaseTimer.unref?.(); }; + let attemptCancellationRequested = false; + const codexAppServerRecoveryRetryAvailable = hasCodexAppServerRecoveryRetryBudget({ + alreadyRetried: codexAppServerRecoveryRetries > 0, + runLoopIterations, + maxRunLoopIterations: MAX_RUN_LOOP_ITERATIONS, + }); const rawAttempt = await runEmbeddedAttemptWithBackend({ sessionId: activeSessionId, sessionKey: resolvedSessionKey, @@ -2148,6 +2158,7 @@ async function runEmbeddedAgentInternal( requestedModelId, fallbackActive: modelId !== requestedModelId || Boolean(resolveRuntimeFallbackReason()), fallbackReason: resolveRuntimeFallbackReason(), + isFinalFallbackAttempt: params.isFinalFallbackAttempt, // Use the harness selected before model/auth setup for the actual // attempt too. Otherwise plugin-owned transports can skip OpenClaw auth // bootstrap but drift back to OpenClaw when the attempt is created. @@ -2210,12 +2221,16 @@ async function runEmbeddedAgentInternal( ? undefined : startLaneProgressHeartbeat, onAttemptTimeout: pluginHarnessOwnsTransport ? undefined : armAttemptTimeoutRelease, - onAttemptAbort: pluginHarnessOwnsTransport - ? undefined - : () => { - stopLaneProgressHeartbeat(); - laneTaskAbortController.abort(); - }, + onAttemptAbort: () => { + attemptCancellationRequested = true; + if (!params.abortSignal?.aborted) { + params.replyOperation?.abortByUser(); + } + if (!pluginHarnessOwnsTransport) { + stopLaneProgressHeartbeat(); + laneTaskAbortController.abort(); + } + }, replyOperation: params.replyOperation, shouldEmitToolResult: params.shouldEmitToolResult, shouldEmitToolOutput: params.shouldEmitToolOutput, @@ -2284,6 +2299,10 @@ async function runEmbeddedAgentInternal( throw postCompactionAbortError; } const attempt = normalizeEmbeddedRunAttemptResult(rawAttempt); + if (attemptCancellationRequested) { + throwIfAborted(); + throw createAgentRunDirectAbortError(); + } const { aborted, @@ -3020,9 +3039,10 @@ async function runEmbeddedAgentInternal( // Retry replay-safe Codex app-server failures. const codexAppServerRecoveryRetry = resolveCodexAppServerRecoveryRetry({ attempt, - alreadyRetried: codexAppServerRecoveryRetries > 0, + retryAvailable: codexAppServerRecoveryRetryAvailable, }); if (codexAppServerRecoveryRetry.retry) { + throwIfAborted(); codexAppServerRecoveryRetries += 1; suppressNextUserMessagePersistence = true; log.warn( diff --git a/src/agents/embedded-agent-runner/run/attempt.ts b/src/agents/embedded-agent-runner/run/attempt.ts index 92e0a675aa9e..16f733f487dc 100644 --- a/src/agents/embedded-agent-runner/run/attempt.ts +++ b/src/agents/embedded-agent-runner/run/attempt.ts @@ -3800,6 +3800,7 @@ export async function runEmbeddedAttempt( cancel: (reason?: "user_abort" | "restart" | "superseded") => void; } = { kind: "embedded", + runId: params.runId, queueMessage: async (text: string, options) => { if (options?.steeringMode) { activeSession.agent.steeringMode = options.steeringMode; diff --git a/src/agents/embedded-agent-runner/run/codex-app-server-recovery.ts b/src/agents/embedded-agent-runner/run/codex-app-server-recovery.ts index 5be48f53ca4a..1db30d383219 100644 --- a/src/agents/embedded-agent-runner/run/codex-app-server-recovery.ts +++ b/src/agents/embedded-agent-runner/run/codex-app-server-recovery.ts @@ -3,6 +3,14 @@ */ import type { EmbeddedRunAttemptResult } from "./types.js"; +export function hasCodexAppServerRecoveryRetryBudget(params: { + alreadyRetried: boolean; + runLoopIterations: number; + maxRunLoopIterations: number; +}): boolean { + return !params.alreadyRetried && params.runLoopIterations < params.maxRunLoopIterations; +} + /** * Decides whether a Codex app-server failure can be retried by replaying the * same turn. The retry is intentionally narrow: stdio-only, replay-safe, once @@ -10,7 +18,7 @@ import type { EmbeddedRunAttemptResult } from "./types.js"; */ export function resolveCodexAppServerRecoveryRetry(params: { attempt: EmbeddedRunAttemptResult; - alreadyRetried: boolean; + retryAvailable: boolean; }): { retry: boolean; reason?: string } { const failure = params.attempt.codexAppServerFailure; if (!failure) { @@ -31,7 +39,7 @@ export function resolveCodexAppServerRecoveryRetry(params: { if (failure.transport !== "stdio") { return { retry: false, reason: "non_stdio_transport" }; } - if (params.alreadyRetried) { + if (!params.retryAvailable) { return { retry: false, reason: "retry_exhausted" }; } if (!failure.replaySafe || !params.attempt.replayMetadata.replaySafe) { diff --git a/src/agents/embedded-agent-runner/runs.test.ts b/src/agents/embedded-agent-runner/runs.test.ts index e69ef4f76efc..cdfdf77814bc 100644 --- a/src/agents/embedded-agent-runner/runs.test.ts +++ b/src/agents/embedded-agent-runner/runs.test.ts @@ -8,6 +8,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { testing as replyRunTesting, createReplyOperation, + isReplyRunActiveForSessionId, } from "../../auto-reply/reply/reply-run-registry.js"; import { setDiagnosticsEnabledForProcess } from "../../infra/diagnostic-events.js"; import { @@ -21,8 +22,10 @@ import { abortAndDrainEmbeddedAgentRun, abortEmbeddedAgentRun, clearActiveEmbeddedRun, + clearEmbeddedAgentRunAbortabilityForRunId, clearEmbeddedRunAbandonment, getActiveEmbeddedRunSnapshot, + isEmbeddedAgentRunAbortableForRunId, isEmbeddedAgentRunAbortableForCompaction, isEmbeddedAgentRunHandleActive, isEmbeddedRunAbandoned, @@ -31,6 +34,7 @@ import { markEmbeddedRunAbandoned, queueEmbeddedAgentMessageWithOutcome, queueEmbeddedAgentMessageWithOutcomeAsync, + retainEmbeddedAgentRunAbortabilityForRunId, resolveActiveEmbeddedRunHandleSessionId, resolveActiveEmbeddedRunHandleSessionIdBySessionFile, setActiveEmbeddedRun, @@ -45,9 +49,11 @@ type RunHandle = Parameters[1]; function createRunHandle( overrides: { abort?: () => void; + isAbortable?: boolean; isCompacting?: boolean; isStreaming?: boolean; isStopped?: () => boolean; + runId?: string; queueMessage?: ( text: string, options?: Parameters[1], @@ -59,9 +65,13 @@ function createRunHandle( // behavior; individual tests supply queue/abort behavior when needed. const abort = overrides.abort ?? (() => {}); return { + runId: overrides.runId, queueMessage: overrides.queueMessage ?? (async () => {}), isStreaming: () => overrides.isStreaming ?? true, ...(overrides.isStopped ? { isStopped: overrides.isStopped } : {}), + ...(overrides.isAbortable !== undefined + ? { isAbortable: () => overrides.isAbortable !== false } + : {}), isCompacting: () => overrides.isCompacting ?? false, supportsTranscriptCommitWait: overrides.supportsTranscriptCommitWait, abort, @@ -124,6 +134,99 @@ describe("embedded-agent runner run registry", () => { expect(abortB).toHaveBeenCalledTimes(1); }); + it("keeps finalizing runs active while rejecting abort requests", () => { + const abort = vi.fn(); + const handle = createRunHandle({ abort, isAbortable: false }); + const operation = createReplyOperation({ + sessionKey: "agent:main:finalizing", + sessionId: "session-finalizing", + resetTriggered: false, + }); + const replyBackend = { + kind: "embedded" as const, + cancel: handle.abort, + isStreaming: handle.isStreaming, + isAbortable: handle.isAbortable, + }; + operation.setPhase("running"); + operation.attachBackend(replyBackend); + setActiveEmbeddedRun("session-finalizing", handle); + + expect(abortEmbeddedAgentRun("session-finalizing")).toBe(false); + expect(abortEmbeddedAgentRun(undefined, { mode: "all" })).toBe(false); + expect(isEmbeddedAgentRunAbortableForCompaction("session-finalizing")).toBe(true); + expect(isEmbeddedAgentRunHandleActive("session-finalizing")).toBe(true); + expect(operation.result).toBeNull(); + expect(abort).not.toHaveBeenCalled(); + + clearActiveEmbeddedRun("session-finalizing", handle); + operation.detachBackend(replyBackend); + expect(abortEmbeddedAgentRun(undefined, { mode: "all" })).toBe(true); + expect(operation.result).toEqual({ kind: "aborted", code: "aborted_for_restart" }); + operation.complete(); + expect(isEmbeddedAgentRunHandleActive("session-finalizing")).toBe(false); + }); + + it("keeps frozen run ownership through forced in-process restart", () => { + const abort = vi.fn(); + const handle = createRunHandle({ abort, isAbortable: false }); + const operation = createReplyOperation({ + sessionKey: "agent:main:restart-finalizing", + sessionId: "session-restart-finalizing", + resetTriggered: false, + }); + const replyBackend = { + kind: "embedded" as const, + cancel: handle.abort, + isStreaming: handle.isStreaming, + isAbortable: handle.isAbortable, + }; + operation.setPhase("running"); + operation.attachBackend(replyBackend); + setActiveEmbeddedRun("session-restart-finalizing", handle); + + expect(abortEmbeddedAgentRun(undefined, { mode: "all", reason: "restart" })).toBe(false); + expect(isEmbeddedAgentRunHandleActive("session-restart-finalizing")).toBe(true); + expect(isReplyRunActiveForSessionId("session-restart-finalizing")).toBe(true); + expect(operation.result).toBeNull(); + expect(abort).not.toHaveBeenCalled(); + + clearActiveEmbeddedRun("session-restart-finalizing", handle); + operation.detachBackend(replyBackend); + operation.complete(); + expect(isEmbeddedAgentRunHandleActive("session-restart-finalizing")).toBe(false); + expect(isReplyRunActiveForSessionId("session-restart-finalizing")).toBe(false); + }); + + it("binds abortability to the owning run id", () => { + const finalizing = createRunHandle({ + abort: vi.fn(), + isAbortable: false, + runId: "run-finalizing", + }); + setActiveEmbeddedRun("session-shared", finalizing); + + expect(isEmbeddedAgentRunAbortableForRunId("run-finalizing")).toBe(false); + expect(isEmbeddedAgentRunAbortableForRunId("run-queued")).toBe(true); + + clearActiveEmbeddedRun("session-shared", finalizing); + expect(isEmbeddedAgentRunAbortableForRunId("run-finalizing")).toBe(true); + + retainEmbeddedAgentRunAbortabilityForRunId("run-finalizing"); + setActiveEmbeddedRun("session-shared", finalizing); + clearActiveEmbeddedRun("session-shared", finalizing); + expect(isEmbeddedAgentRunAbortableForRunId("run-finalizing")).toBe(false); + + const queued = createRunHandle({ runId: "run-queued" }); + setActiveEmbeddedRun("session-shared", queued); + + expect(isEmbeddedAgentRunAbortableForRunId("run-finalizing")).toBe(false); + expect(isEmbeddedAgentRunAbortableForRunId("run-queued")).toBe(true); + + clearEmbeddedAgentRunAbortabilityForRunId("run-finalizing"); + expect(isEmbeddedAgentRunAbortableForRunId("run-finalizing")).toBe(true); + }); + it("passes restart ownership to every aborted run", () => { const abort = vi.fn(); setActiveEmbeddedRun("session-restart", createRunHandle({ abort })); @@ -132,6 +235,106 @@ describe("embedded-agent runner run registry", () => { expect(abort).toHaveBeenCalledWith("restart"); }); + it("claims shared restart ownership before invoking an attached handle", () => { + const abort = vi.fn(); + const handle = createRunHandle({ abort }); + const operation = createReplyOperation({ + sessionKey: "agent:main:restart-owned", + sessionId: "session-restart-owned", + resetTriggered: false, + }); + operation.setPhase("running"); + operation.attachBackend({ + kind: "embedded", + cancel: handle.abort, + isStreaming: handle.isStreaming, + isAbortable: handle.isAbortable, + }); + setActiveEmbeddedRun("session-restart-owned", handle); + + expect(abortEmbeddedAgentRun(undefined, { mode: "all", reason: "restart" })).toBe(true); + expect(operation.result).toEqual({ kind: "aborted", code: "aborted_for_restart" }); + expect(abort).toHaveBeenCalledTimes(1); + expect(abort).toHaveBeenCalledWith("restart"); + }); + + it.each(["all", "compacting"] as const)( + "does not bypass frozen shared ownership through %s handle aborts", + (mode) => { + const abort = vi.fn(); + const handle = createRunHandle({ abort, isCompacting: true }); + const sessionId = `session-restart-frozen-${mode}`; + const operation = createReplyOperation({ + sessionKey: `agent:main:restart-frozen-${mode}`, + sessionId, + resetTriggered: false, + }); + operation.setPhase("running"); + operation.attachBackend({ + kind: "embedded", + cancel: handle.abort, + isStreaming: handle.isStreaming, + isAbortable: handle.isAbortable, + isCompacting: handle.isCompacting, + }); + operation.freezeAbort(); + setActiveEmbeddedRun(sessionId, handle); + + expect(abortEmbeddedAgentRun(undefined, { mode, reason: "restart" })).toBe(false); + expect(operation.result).toBeNull(); + expect(abort).not.toHaveBeenCalled(); + }, + ); + + it("keeps shared restart ownership when the attached cancel callback throws", () => { + const abort = vi.fn(() => { + throw new Error("cancel failed"); + }); + const handle = createRunHandle({ abort }); + const operation = createReplyOperation({ + sessionKey: "agent:main:restart-throwing", + sessionId: "session-restart-throwing", + resetTriggered: false, + }); + operation.setPhase("running"); + operation.attachBackend({ + kind: "embedded", + cancel: handle.abort, + isStreaming: handle.isStreaming, + isAbortable: handle.isAbortable, + }); + setActiveEmbeddedRun("session-restart-throwing", handle); + + expect(abortEmbeddedAgentRun(undefined, { mode: "all", reason: "restart" })).toBe(true); + expect(operation.result).toEqual({ kind: "aborted", code: "aborted_for_restart" }); + expect(abort).toHaveBeenCalledTimes(1); + }); + + it("does not bypass retained terminal ownership through compacting handle aborts", () => { + const abort = vi.fn(); + const handle = createRunHandle({ abort, isCompacting: true }); + const operation = createReplyOperation({ + sessionKey: "agent:main:restart-failed-compacting", + sessionId: "session-restart-failed-compacting", + resetTriggered: false, + }); + operation.setPhase("running"); + operation.attachBackend({ + kind: "embedded", + cancel: handle.abort, + isStreaming: handle.isStreaming, + isAbortable: handle.isAbortable, + isCompacting: handle.isCompacting, + }); + operation.retainFailureUntilComplete(); + operation.fail("run_failed", new Error("terminal failure")); + setActiveEmbeddedRun("session-restart-failed-compacting", handle); + + expect(abortEmbeddedAgentRun(undefined, { mode: "compacting", reason: "restart" })).toBe(false); + expect(operation.result).toMatchObject({ kind: "failed", code: "run_failed" }); + expect(abort).not.toHaveBeenCalled(); + }); + it("resolves active embedded runs by canonical session file", async () => { // Session-file lookup canonicalizes symlinks so heartbeat/diagnostic callers // can find the active handle from the file path they observe. diff --git a/src/agents/embedded-agent-runner/runs.ts b/src/agents/embedded-agent-runner/runs.ts index 98956d1c6f70..fe28aa2a8af7 100644 --- a/src/agents/embedded-agent-runner/runs.ts +++ b/src/agents/embedded-agent-runner/runs.ts @@ -8,6 +8,7 @@ import { isReplyRunActiveForSessionId, isReplyRunAbortableForCompaction, isReplyRunStreamingForSessionId, + listActiveReplyRunSessionIds, queueReplyRunMessage, waitForReplyRunEndBySessionId, } from "../../auto-reply/reply/reply-run-registry.js"; @@ -24,6 +25,7 @@ import { import { resolveTimerTimeoutMs } from "../../shared/number-coercion.js"; import { ACTIVE_EMBEDDED_RUNS, + ACTIVE_EMBEDDED_RUNS_BY_RUN_ID, ACTIVE_EMBEDDED_RUN_SESSION_IDS_BY_FILE, ACTIVE_EMBEDDED_RUN_SESSION_IDS_BY_KEY, ACTIVE_EMBEDDED_RUN_SNAPSHOTS, @@ -32,6 +34,7 @@ import { ABANDONED_EMBEDDED_RUN_SESSION_IDS_BY_KEY, EMBEDDED_RUN_WAITERS, getActiveEmbeddedRunCount, + RETAINED_EMBEDDED_RUN_ABORTABILITY_RUN_IDS, type ActiveEmbeddedRunSnapshot, type AbandonedEmbeddedRun, type EmbeddedAgentQueueHandle, @@ -347,6 +350,61 @@ function isEmbeddedQueueHandleMessageInjectable( } } +function isEmbeddedRunHandleAbortable( + sessionId: string, + handle: EmbeddedAgentQueueHandle, +): boolean { + try { + return handle.isAbortable?.() !== false; + } catch (err) { + diag.warn( + `abort failed: sessionId=${sessionId} reason=abortable_check_failed err=${String(err)}`, + ); + return false; + } +} + +export function isEmbeddedAgentRunAbortableForRunId(runId: string): boolean { + const normalizedRunId = runId.trim(); + if (!normalizedRunId) { + return true; + } + const handle = ACTIVE_EMBEDDED_RUNS_BY_RUN_ID.get(normalizedRunId); + return handle ? isEmbeddedRunHandleAbortable(normalizedRunId, handle) : true; +} + +export function clearEmbeddedAgentRunAbortabilityForRunId(runId: string): void { + const normalizedRunId = runId.trim(); + if (normalizedRunId) { + ACTIVE_EMBEDDED_RUNS_BY_RUN_ID.delete(normalizedRunId); + RETAINED_EMBEDDED_RUN_ABORTABILITY_RUN_IDS.delete(normalizedRunId); + } +} + +export function retainEmbeddedAgentRunAbortabilityForRunId(runId: string): void { + const normalizedRunId = runId.trim(); + if (normalizedRunId) { + RETAINED_EMBEDDED_RUN_ABORTABILITY_RUN_IDS.add(normalizedRunId); + } +} + +function clearEmbeddedRunAbortability( + handle: EmbeddedAgentQueueHandle, + opts?: { retainFinalizing?: boolean }, +): void { + if (!handle.runId || ACTIVE_EMBEDDED_RUNS_BY_RUN_ID.get(handle.runId) !== handle) { + return; + } + if ( + opts?.retainFinalizing && + RETAINED_EMBEDDED_RUN_ABORTABILITY_RUN_IDS.has(handle.runId) && + !isEmbeddedRunHandleAbortable(handle.runId, handle) + ) { + return; + } + ACTIVE_EMBEDDED_RUNS_BY_RUN_ID.delete(handle.runId); +} + export async function queueEmbeddedAgentMessageWithOutcomeAsync( sessionId: string, text: string, @@ -465,6 +523,10 @@ export function abortEmbeddedAgentRun( diag.debug(`abort failed: sessionId=${sessionId} reason=no_active_run`); return false; } + if (!isEmbeddedRunHandleAbortable(sessionId, handle)) { + diag.debug(`abort failed: sessionId=${sessionId} reason=not_abortable`); + return false; + } diag.debug(`aborting run: sessionId=${sessionId}`); try { handle.abort(opts?.reason); @@ -478,12 +540,19 @@ export function abortEmbeddedAgentRun( const abortActiveEmbeddedRunHandles = (params: { shouldAbort: (handle: EmbeddedAgentQueueHandle) => boolean; formatDebugMessage: (sessionId: string) => string; + skipSessionIds?: ReadonlySet; }): boolean => { let aborted = false; for (const [id, handle] of ACTIVE_EMBEDDED_RUNS) { + if (params.skipSessionIds?.has(id)) { + continue; + } if (!params.shouldAbort(handle)) { continue; } + if (!isEmbeddedRunHandleAbortable(id, handle)) { + continue; + } diag.debug(params.formatDebugMessage(id)); try { handle.abort(opts?.reason); @@ -497,19 +566,33 @@ export function abortEmbeddedAgentRun( const mode = opts?.mode; if (mode === "compacting") { + const replyOwnedSessionIds = new Set(listActiveReplyRunSessionIds()); + const replyAborted = abortActiveReplyRuns({ + mode, + onAbortError: (id, err) => + diag.warn(`abort failed: sessionId=${id} owner=reply_run err=${String(err)}`), + }); const aborted = abortActiveEmbeddedRunHandles({ shouldAbort: (handle) => handle.isCompacting(), formatDebugMessage: (id) => `aborting compacting run: sessionId=${id}`, + skipSessionIds: replyOwnedSessionIds, }); - return abortActiveReplyRuns({ mode }) || aborted; + return replyAborted || aborted; } if (mode === "all") { + const replyOwnedSessionIds = new Set(listActiveReplyRunSessionIds()); + const replyAborted = abortActiveReplyRuns({ + mode, + onAbortError: (id, err) => + diag.warn(`abort failed: sessionId=${id} owner=reply_run err=${String(err)}`), + }); const aborted = abortActiveEmbeddedRunHandles({ shouldAbort: () => true, formatDebugMessage: (id) => `aborting run: sessionId=${id}`, + skipSessionIds: replyOwnedSessionIds, }); - return abortActiveReplyRuns({ mode }) || aborted; + return replyAborted || aborted; } return false; @@ -532,9 +615,10 @@ export function isEmbeddedAgentRunHandleActive(sessionId: string): boolean { } export function isEmbeddedAgentRunAbortableForCompaction(sessionId: string): boolean { - const active = ACTIVE_EMBEDDED_RUNS.has(sessionId) || isReplyRunAbortableForCompaction(sessionId); + const handle = ACTIVE_EMBEDDED_RUNS.get(sessionId); + const active = handle ? true : isReplyRunAbortableForCompaction(sessionId); if (active) { - diag.debug(`run compact abort check: sessionId=${sessionId} active=true`); + diag.debug(`run compact coordination check: sessionId=${sessionId} active=true`); } return active; } @@ -699,9 +783,16 @@ export function setActiveEmbeddedRun( sessionKey?: string, sessionFile?: string, ) { - const wasActive = ACTIVE_EMBEDDED_RUNS.has(sessionId); + const previousHandle = ACTIVE_EMBEDDED_RUNS.get(sessionId); + const wasActive = previousHandle !== undefined; + if (previousHandle) { + clearEmbeddedRunAbortability(previousHandle, { retainFinalizing: true }); + } clearEmbeddedRunAbandonment({ sessionId, sessionKey, sessionFile }); ACTIVE_EMBEDDED_RUNS.set(sessionId, handle); + if (handle.runId) { + ACTIVE_EMBEDDED_RUNS_BY_RUN_ID.set(handle.runId, handle); + } setActiveRunSessionKey(sessionKey, sessionId); clearActiveRunSessionFiles(sessionId); setActiveRunSessionFile(sessionFile, sessionId); @@ -745,6 +836,7 @@ export function clearActiveEmbeddedRun( handle: EmbeddedAgentQueueHandle, sessionKey?: string, sessionFile?: string, + reason = "run_completed", ) { const activeHandle = ACTIVE_EMBEDDED_RUNS.get(sessionId); if (activeHandle === undefined) { @@ -752,6 +844,7 @@ export function clearActiveEmbeddedRun( } if (activeHandle === handle) { ACTIVE_EMBEDDED_RUNS.delete(sessionId); + clearEmbeddedRunAbortability(handle, { retainFinalizing: true }); ACTIVE_EMBEDDED_RUN_SNAPSHOTS.delete(sessionId); clearActiveRunSessionKeys(sessionId, sessionKey); clearActiveRunSessionFiles(sessionId, sessionFile); @@ -760,7 +853,7 @@ export function clearActiveEmbeddedRun( sessionKey, sessionFile, state: "idle", - reason: "run_completed", + reason, }); markDiagnosticEmbeddedRunEnded({ sessionId, sessionKey }); if (!sessionId.startsWith("probe-")) { @@ -778,8 +871,10 @@ export function forceClearEmbeddedAgentRun( reason = "stuck_recovery", ): boolean { let cleared = false; - if (ACTIVE_EMBEDDED_RUNS.has(sessionId)) { + const handle = ACTIVE_EMBEDDED_RUNS.get(sessionId); + if (handle) { ACTIVE_EMBEDDED_RUNS.delete(sessionId); + clearEmbeddedRunAbortability(handle); ACTIVE_EMBEDDED_RUN_SNAPSHOTS.delete(sessionId); clearActiveRunSessionKeys(sessionId, sessionKey); clearActiveRunSessionFiles(sessionId); @@ -802,6 +897,8 @@ export const testing = { } EMBEDDED_RUN_WAITERS.clear(); ACTIVE_EMBEDDED_RUNS.clear(); + ACTIVE_EMBEDDED_RUNS_BY_RUN_ID.clear(); + RETAINED_EMBEDDED_RUN_ABORTABILITY_RUN_IDS.clear(); ACTIVE_EMBEDDED_RUN_SNAPSHOTS.clear(); ACTIVE_EMBEDDED_RUN_SESSION_IDS_BY_KEY.clear(); ACTIVE_EMBEDDED_RUN_SESSION_IDS_BY_FILE.clear(); diff --git a/src/agents/model-fallback.run-embedded.e2e.test.ts b/src/agents/model-fallback.run-embedded.e2e.test.ts index bf55b2651d46..88955786b6e5 100644 --- a/src/agents/model-fallback.run-embedded.e2e.test.ts +++ b/src/agents/model-fallback.run-embedded.e2e.test.ts @@ -245,6 +245,7 @@ async function runEmbeddedFallback(params: { sessionId: params.sessionId, lane: params.lane, agentDir: params.agentDir, + abortSignal: params.abortSignal, run: (provider, model, options) => runEmbeddedAgent({ sessionId, @@ -259,6 +260,7 @@ async function runEmbeddedFallback(params: { lane: params.lane, authProfileIdSource: "auto", allowTransientCooldownProbe: options?.allowTransientCooldownProbe, + isFinalFallbackAttempt: options?.isFinalFallbackAttempt, timeoutMs: 5_000, runId: params.runId, abortSignal: params.abortSignal, diff --git a/src/agents/model-fallback.test.ts b/src/agents/model-fallback.test.ts index 043d747353a2..a607d0bc7d3c 100644 --- a/src/agents/model-fallback.test.ts +++ b/src/agents/model-fallback.test.ts @@ -31,7 +31,10 @@ import { runWithImageModelFallback, runWithModelFallback as runWithModelFallbackBase, } from "./model-fallback.js"; -import { createAgentRunRestartAbortError } from "./run-termination.js"; +import { + createAgentRunDirectAbortError, + createAgentRunRestartAbortError, +} from "./run-termination.js"; import { SessionWriteLockTimeoutError } from "./session-write-lock-error.js"; import { makeModelFallbackCfg } from "./test-helpers/model-fallback-config-fixture.js"; @@ -2800,6 +2803,25 @@ describe("runWithModelFallback", () => { expect(run).toHaveBeenCalledTimes(1); }); + it("does not fall back on direct active-run aborts without an aborted signal", async () => { + const cfg = makeCfg(); + const run = vi + .fn() + .mockRejectedValueOnce(createAgentRunDirectAbortError()) + .mockResolvedValueOnce("fallback should not run"); + + await expect( + runWithModelFallback({ + cfg, + provider: "openai", + model: "gpt-4.1-mini", + run, + }), + ).rejects.toThrow("agent run aborted"); + + expect(run).toHaveBeenCalledTimes(1); + }); + it("does not fall back when user cancels with AbortError reason", async () => { const cfg = makeCfg(); const controller = new AbortController(); diff --git a/src/agents/model-fallback.ts b/src/agents/model-fallback.ts index 8ba71752eaee..9514d6b81be5 100644 --- a/src/agents/model-fallback.ts +++ b/src/agents/model-fallback.ts @@ -75,7 +75,7 @@ import { resolveConfiguredModelRef, resolveModelRefFromString, } from "./model-selection-resolve.js"; -import { isAgentRunRestartAbortReason } from "./run-termination.js"; +import { isAgentRunDirectAbortReason, isAgentRunRestartAbortReason } from "./run-termination.js"; import { resolveSessionSuspensionReason, runWithDeferredSessionSuspension, @@ -354,7 +354,7 @@ async function runFallbackCandidate(params: { if (isTerminalAbort(params.abortSignal) || isCallerAbortSignal(params.abortSignal)) { throw err; } - if (isAgentRunRestartAbortReason(err)) { + if (isAgentRunDirectAbortReason(err) || isAgentRunRestartAbortReason(err)) { throw err; } // Normalize abort-wrapped rate-limit errors (e.g. Google Vertex RESOURCE_EXHAUSTED) @@ -1308,6 +1308,7 @@ function shouldDiscardDeferredSessionSuspension(params: { return ( isTerminalAbort(params.abortSignal) || isCallerAbortSignal(params.abortSignal) || + isAgentRunDirectAbortReason(params.error) || isAgentRunRestartAbortReason(params.error) || isCommandLaneTaskTimeoutError(params.error) || isNonProviderRuntimeCoordinationError(params.error) || diff --git a/src/agents/run-termination.test.ts b/src/agents/run-termination.test.ts index 2baec4a077fa..824748552b9e 100644 --- a/src/agents/run-termination.test.ts +++ b/src/agents/run-termination.test.ts @@ -1,6 +1,8 @@ import { describe, expect, it } from "vitest"; import { + createAgentRunDirectAbortError, createAgentRunRestartAbortError, + isAgentRunDirectAbortReason, isAbortedAgentStopReason, resolveAgentRunAbortLifecycleFields, } from "./run-termination.js"; @@ -43,4 +45,15 @@ describe("resolveAgentRunAbortLifecycleFields", () => { expect(isAbortedAgentStopReason("restart")).toBe(true); expect(isAbortedAgentStopReason("timeout")).toBe(false); }); + + it("marks direct active-run cancellation independently of an AbortSignal", () => { + const error = createAgentRunDirectAbortError(); + + expect(error).toMatchObject({ + name: "AbortError", + message: "agent run aborted", + }); + expect(isAgentRunDirectAbortReason(error)).toBe(true); + expect(isAgentRunDirectAbortReason(createAgentRunRestartAbortError())).toBe(false); + }); }); diff --git a/src/agents/run-termination.ts b/src/agents/run-termination.ts index 79b7c3451005..c5b363a79583 100644 --- a/src/agents/run-termination.ts +++ b/src/agents/run-termination.ts @@ -11,6 +11,20 @@ export const AGENT_RUN_ABORTED_ERROR = "agent run aborted" as const; export const AGENT_RUN_RESTART_ABORT_STOP_REASON = "restart" as const; const AGENT_RUN_RESTART_ABORT_ERROR_CODE = "OPENCLAW_RESTART_ABORT"; +const AGENT_RUN_DIRECT_ABORT_ERROR_CODE = "OPENCLAW_DIRECT_ABORT"; + +export function createAgentRunDirectAbortError(): Error { + const error = new Error(AGENT_RUN_ABORTED_ERROR) as Error & { code: string }; + error.name = "AbortError"; + error.code = AGENT_RUN_DIRECT_ABORT_ERROR_CODE; + return error; +} + +export function isAgentRunDirectAbortReason(value: unknown): boolean { + return ( + value instanceof Error && "code" in value && value.code === AGENT_RUN_DIRECT_ABORT_ERROR_CODE + ); +} export function createAgentRunRestartAbortError(): Error { const error = new Error("agent run aborted for restart") as Error & { code: string }; diff --git a/src/agents/subagent-control.runtime.ts b/src/agents/subagent-control.runtime.ts index a61c1bd663fc..696d94496dc9 100644 --- a/src/agents/subagent-control.runtime.ts +++ b/src/agents/subagent-control.runtime.ts @@ -2,4 +2,4 @@ * Runtime seams used by subagent control for queue and embedded-run cancellation. */ export { clearSessionQueues } from "../auto-reply/reply/queue.js"; -export { abortEmbeddedAgentRun } from "./embedded-agent-runner/runs.js"; +export { abortEmbeddedAgentRun, isEmbeddedAgentRunActive } from "./embedded-agent-runner/runs.js"; diff --git a/src/agents/subagent-control.test.ts b/src/agents/subagent-control.test.ts index c8875dbaf29b..76ba5e82611b 100644 --- a/src/agents/subagent-control.test.ts +++ b/src/agents/subagent-control.test.ts @@ -125,6 +125,7 @@ function setSubagentControlDepsForTest( // while swapping process-owned queues and embedded-run aborts for fakes. testing.setDepsForTest({ abortEmbeddedAgentRun: () => false, + isEmbeddedAgentRunActive: () => false, clearSessionQueues: () => ({ followupCleared: 0, laneCleared: 0, keys: [] }), patchSessionEntry: async ( scope: SessionAccessScope, @@ -603,6 +604,46 @@ describe("killSubagentRunAdmin", () => { expect(result).toEqual({ found: false, killed: false }); }); + it("does not mark a finalizing run killed when its abort is rejected", async () => { + const childSessionKey = "agent:main:subagent:worker-finalizing"; + const storePath = writeSessionStoreFixture("admin-kill-finalizing", { + [childSessionKey]: { + sessionId: "sess-worker-finalizing", + updatedAt: Date.now(), + }, + }); + + addSubagentRunForTests({ + runId: "run-worker-finalizing", + childSessionKey, + controllerSessionKey: "agent:main:other-controller", + requesterSessionKey: "agent:main:other-requester", + requesterDisplayKey: "other-requester", + task: "finish the reply", + cleanup: "keep", + createdAt: Date.now() - 5_000, + startedAt: Date.now() - 4_000, + }); + setSubagentControlDepsForTest({ + isEmbeddedAgentRunActive: () => true, + abortEmbeddedAgentRun: () => false, + }); + + const result = await killSubagentRunAdmin({ + cfg: cfgWithSessionStore(storePath), + sessionKey: childSessionKey, + }); + + expect(result.found).toBe(true); + expect(result.killed).toBe(false); + expect(getSubagentRunByChildSessionKey(childSessionKey)?.endedAt).toBeUndefined(); + const persisted = JSON.parse(fs.readFileSync(storePath, "utf-8")) as Record< + string, + { abortedLastRun?: boolean } + >; + expect(persisted[childSessionKey]?.abortedLastRun).toBeUndefined(); + }); + it("does not kill a newest finished run when only a stale older row is still active", async () => { const childSessionKey = "agent:main:subagent:worker-stale-admin"; @@ -1264,6 +1305,70 @@ describe("steerControlledSubagentRun", () => { } }); + it("does not replace a finalizing run when its abort is rejected", async () => { + const childSessionKey = "agent:main:subagent:steer-finalizing"; + const storePath = writeSessionStoreFixture("steer-finalizing", { + [childSessionKey]: { + sessionId: "sess-steer-finalizing", + updatedAt: Date.now(), + }, + }); + addSubagentRunForTests({ + runId: "run-steer-finalizing", + childSessionKey, + controllerSessionKey: "agent:main:main", + requesterSessionKey: "agent:main:main", + requesterDisplayKey: "main", + task: "finish the reply", + cleanup: "keep", + createdAt: Date.now() - 5_000, + startedAt: Date.now() - 4_000, + }); + const callGateway = vi.fn(async () => { + throw new Error("gateway should not be called"); + }); + setSubagentControlDepsForTest({ + callGateway, + isEmbeddedAgentRunActive: () => true, + abortEmbeddedAgentRun: () => false, + }); + + const result = await steerControlledSubagentRun({ + cfg: cfgWithSessionStore(storePath), + controller: { + controllerSessionKey: "agent:main:main", + callerSessionKey: "agent:main:main", + callerIsSubagent: false, + controlScope: "children", + }, + entry: { + runId: "run-steer-finalizing", + childSessionKey, + requesterSessionKey: "agent:main:main", + requesterDisplayKey: "main", + controllerSessionKey: "agent:main:main", + task: "finish the reply", + cleanup: "keep", + createdAt: Date.now() - 5_000, + startedAt: Date.now() - 4_000, + }, + message: "new direction", + }); + + expect(result).toEqual({ + status: "error", + runId: "run-steer-finalizing", + sessionKey: childSessionKey, + sessionId: "sess-steer-finalizing", + error: "Subagent reply is already finalizing and can no longer be restarted.", + }); + expect(callGateway).not.toHaveBeenCalled(); + const storedRun = getSubagentRunByChildSessionKey(childSessionKey); + expect(storedRun?.runId).toBe("run-steer-finalizing"); + expect(storedRun?.endedAt).toBeUndefined(); + expect(storedRun?.suppressAnnounceReason).toBeUndefined(); + }); + it("rejects steering runs that are no longer tracked in the registry", async () => { setSubagentControlDepsForTest({ callGateway: async () => { diff --git a/src/agents/subagent-control.ts b/src/agents/subagent-control.ts index 8bb555a8a7ff..3885fce8bc19 100644 --- a/src/agents/subagent-control.ts +++ b/src/agents/subagent-control.ts @@ -52,6 +52,7 @@ const steerRateLimit = new Map(); type GatewayCaller = typeof callGateway; type PatchSessionEntry = typeof patchSessionEntry; type AbortEmbeddedAgentRun = (sessionId: string) => boolean; +type IsEmbeddedAgentRunActive = (sessionId: string) => boolean; type ClearSessionQueues = (keys: Array) => ClearSessionQueueResult; const defaultSubagentControlDeps = { @@ -63,6 +64,7 @@ let subagentControlDeps: { callGateway: GatewayCaller; patchSessionEntry: PatchSessionEntry; abortEmbeddedAgentRun?: AbortEmbeddedAgentRun; + isEmbeddedAgentRunActive?: IsEmbeddedAgentRunActive; clearSessionQueues?: ClearSessionQueues; } = defaultSubagentControlDeps; @@ -76,11 +78,17 @@ function loadSubagentControlRuntime() { async function resolveSubagentControlRuntime(): Promise<{ abortEmbeddedAgentRun: AbortEmbeddedAgentRun; + isEmbeddedAgentRunActive: IsEmbeddedAgentRunActive; clearSessionQueues: ClearSessionQueues; }> { - if (subagentControlDeps.abortEmbeddedAgentRun && subagentControlDeps.clearSessionQueues) { + if ( + subagentControlDeps.abortEmbeddedAgentRun && + subagentControlDeps.isEmbeddedAgentRunActive && + subagentControlDeps.clearSessionQueues + ) { return { abortEmbeddedAgentRun: subagentControlDeps.abortEmbeddedAgentRun, + isEmbeddedAgentRunActive: subagentControlDeps.isEmbeddedAgentRunActive, clearSessionQueues: subagentControlDeps.clearSessionQueues, }; } @@ -88,6 +96,8 @@ async function resolveSubagentControlRuntime(): Promise<{ return { abortEmbeddedAgentRun: subagentControlDeps.abortEmbeddedAgentRun ?? runtime.abortEmbeddedAgentRun, + isEmbeddedAgentRunActive: + subagentControlDeps.isEmbeddedAgentRunActive ?? runtime.isEmbeddedAgentRunActive, clearSessionQueues: subagentControlDeps.clearSessionQueues ?? runtime.clearSessionQueues, }; } @@ -184,6 +194,7 @@ async function killSubagentRun(params: { }); const sessionId = resolved.entry?.sessionId; const runtime = await resolveSubagentControlRuntime(); + const active = sessionId ? runtime.isEmbeddedAgentRunActive(sessionId) : false; const aborted = sessionId ? runtime.abortEmbeddedAgentRun(sessionId) : false; const cleared = runtime.clearSessionQueues([childSessionKey, sessionId]); if (cleared.followupCleared > 0 || cleared.laneCleared > 0) { @@ -191,6 +202,9 @@ async function killSubagentRun(params: { `subagents control kill: cleared followups=${cleared.followupCleared} lane=${cleared.laneCleared} keys=${cleared.keys.join(",")}`, ); } + if (active && !aborted) { + return { killed: false, sessionId }; + } if (resolved.entry) { try { await subagentControlDeps.patchSessionEntry( @@ -548,12 +562,22 @@ export async function steerControlledSubagentRun(params: { ? targetSession.entry.sessionId.trim() : undefined; const restartSessionId = sessionId ? crypto.randomUUID() : undefined; + const runtime = await resolveSubagentControlRuntime(); if (sessionId) { - const runtime = await resolveSubagentControlRuntime(); - runtime.abortEmbeddedAgentRun(sessionId); + const active = runtime.isEmbeddedAgentRunActive(sessionId); + const aborted = runtime.abortEmbeddedAgentRun(sessionId); + if (active && !aborted) { + clearSubagentRunSteerRestart(params.entry.runId); + return { + status: "error", + runId: params.entry.runId, + sessionKey: params.entry.childSessionKey, + sessionId, + error: "Subagent reply is already finalizing and can no longer be restarted.", + }; + } } - const runtime = await resolveSubagentControlRuntime(); const cleared = runtime.clearSessionQueues([params.entry.childSessionKey, sessionId]); if (cleared.followupCleared > 0 || cleared.laneCleared > 0) { logVerbose( @@ -740,6 +764,7 @@ export const testing = { callGateway: GatewayCaller; patchSessionEntry: PatchSessionEntry; abortEmbeddedAgentRun: AbortEmbeddedAgentRun; + isEmbeddedAgentRunActive: IsEmbeddedAgentRunActive; clearSessionQueues: ClearSessionQueues; }>, ) { diff --git a/src/auto-reply/reply/abort.runtime-types.ts b/src/auto-reply/reply/abort.runtime-types.ts index 5a9e9fadcbec..ba5a820f8f35 100644 --- a/src/auto-reply/reply/abort.runtime-types.ts +++ b/src/auto-reply/reply/abort.runtime-types.ts @@ -6,6 +6,7 @@ import type { FinalizedMsgContext } from "../templating.js"; type FastAbortResult = { handled: boolean; aborted: boolean; + rejectionReason?: "finalizing"; stoppedSubagents?: number; }; @@ -16,4 +17,7 @@ export type TryFastAbortFromMessage = (params: { }) => Promise; /** Formats the user-visible abort acknowledgement text. */ -export type FormatAbortReplyText = (stoppedSubagents?: number) => string; +export type FormatAbortReplyText = ( + stoppedSubagents?: number, + rejectionReason?: FastAbortResult["rejectionReason"], +) => string; diff --git a/src/auto-reply/reply/abort.test.ts b/src/auto-reply/reply/abort.test.ts index 2f0fb1538651..c26422bc0b04 100644 --- a/src/auto-reply/reply/abort.test.ts +++ b/src/auto-reply/reply/abort.test.ts @@ -4,10 +4,11 @@ import path from "node:path"; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import type { SubagentRunRecord } from "../../agents/subagent-registry.js"; import type { OpenClawConfig } from "../../config/config.js"; -import { createSuiteTempRootTracker } from "../../test-helpers/temp-dir.js"; import type { SessionAbortTargetResult } from "../../config/sessions/session-accessor.js"; +import { createSuiteTempRootTracker } from "../../test-helpers/temp-dir.js"; import { testing as abortTesting, + formatAbortReplyText, getAbortMemory, getAbortMemorySizeForTest, isAbortRequestText, @@ -812,6 +813,53 @@ describe("abort detection", () => { }); }); + it("does not report /stop success after the active backend freezes its outcome", async () => { + const sessionKey = "agent:main:telegram:direct:finalizing"; + const sessionId = "session-finalizing"; + const { cfg } = await createAbortConfig({ + sessionIdsByKey: { [sessionKey]: sessionId }, + }); + const cancel = vi.fn(); + const operation = createReplyOperation({ + sessionKey, + sessionId, + resetTriggered: false, + }); + operation.attachBackend({ + kind: "embedded", + cancel, + isStreaming: () => false, + isAbortable: () => false, + }); + operation.setPhase("running"); + runtimeAbortMocks.abortEmbeddedAgentRun.mockReturnValue(false); + runtimeAbortMocks.resolveActiveEmbeddedRunSessionId.mockReturnValue(sessionId); + const markSessionAbortTarget = vi.fn(); + abortTesting.setDepsForTests({ markSessionAbortTarget }); + + const result = await runStopCommand({ + cfg, + sessionKey, + from: "telegram:finalizing", + to: "telegram:finalizing", + }); + + expect(result).toMatchObject({ + handled: true, + aborted: false, + rejectionReason: "finalizing", + }); + expect(operation.result).toBeNull(); + expect(replyRunRegistry.isActive(sessionKey)).toBe(true); + expect(cancel).not.toHaveBeenCalled(); + expect(markSessionAbortTarget).not.toHaveBeenCalled(); + expect(getAbortMemory(sessionKey)).toBeUndefined(); + expect(formatAbortReplyText(undefined, result.rejectionReason)).toBe( + "Agent reply is already finalizing and can no longer be aborted.", + ); + operation.complete(); + }); + it("fast-abort of an ACP target aborts the source stored session when no source reply operation is registered", async () => { const sourceSessionKey = "agent:main:discord:channel:C2"; const acpSessionKey = "agent:codex:acp:bound-session-stored-source"; diff --git a/src/auto-reply/reply/abort.ts b/src/auto-reply/reply/abort.ts index 6582485aec36..717e74f5cfc4 100644 --- a/src/auto-reply/reply/abort.ts +++ b/src/auto-reply/reply/abort.ts @@ -112,12 +112,17 @@ export const testing = { }, }; -export function abortSessionRunTarget(params: { key?: string; sessionId?: string }): boolean { +export function abortSessionRunTargetWithOutcome(params: { key?: string; sessionId?: string }): { + active: boolean; + aborted: boolean; +} { const sessionIds = new Set(); const key = normalizeOptionalString(params.key); + let active = key ? replyRunRegistry.isActive(key) : false; if (key) { const activeSessionId = abortDeps.resolveActiveEmbeddedRunSessionId(key); if (activeSessionId) { + active = true; sessionIds.add(activeSessionId); } } @@ -130,10 +135,25 @@ export function abortSessionRunTarget(params: { key?: string; sessionId?: string for (const sessionId of sessionIds) { aborted = abortDeps.abortEmbeddedAgentRun(sessionId) || aborted; } - return aborted; + return { active, aborted }; } -export function formatAbortReplyText(stoppedSubagents?: number): string { +export function abortSessionRunTarget(params: { key?: string; sessionId?: string }): boolean { + return abortSessionRunTargetWithOutcome(params).aborted; +} + +export function formatAbortReplyText( + stoppedSubagents?: number, + rejectionReason?: "finalizing", +): string { + if (rejectionReason === "finalizing") { + const base = "Agent reply is already finalizing and can no longer be aborted."; + if (typeof stoppedSubagents !== "number" || stoppedSubagents <= 0) { + return base; + } + const label = stoppedSubagents === 1 ? "sub-agent" : "sub-agents"; + return `${base} Stopped ${stoppedSubagents} ${label}.`; + } if (typeof stoppedSubagents !== "number" || stoppedSubagents <= 0) { return "⚙️ Agent was aborted."; } @@ -258,15 +278,23 @@ export function stopSubagentsForRequester(params: { sessionKey: childKey, storePath, })?.sessionId; - const aborted = abortSessionRunTarget({ key: childKey, sessionId }); - const markedTerminated = - abortDeps.markSubagentRunTerminated({ - runId: run.runId, - childSessionKey: childKey, - reason: "killed", - }) > 0; + const abortOutcome = abortSessionRunTargetWithOutcome({ key: childKey, sessionId }); + const abortRejected = abortOutcome.active && !abortOutcome.aborted; + const markedTerminated = abortRejected + ? false + : abortDeps.markSubagentRunTerminated({ + runId: run.runId, + childSessionKey: childKey, + reason: "killed", + }) > 0; - if (markedTerminated || aborted || cleared.followupCleared > 0 || cleared.laneCleared > 0) { + if ( + !abortRejected && + (markedTerminated || + abortOutcome.aborted || + cleared.followupCleared > 0 || + cleared.laneCleared > 0) + ) { stopped += 1; } } @@ -288,7 +316,12 @@ export function stopSubagentsForRequester(params: { export async function tryFastAbortFromMessage(params: { ctx: FinalizedMsgContext; cfg: OpenClawConfig; -}): Promise<{ handled: boolean; aborted: boolean; stoppedSubagents?: number }> { +}): Promise<{ + handled: boolean; + aborted: boolean; + rejectionReason?: "finalizing"; + stoppedSubagents?: number; +}> { const { ctx, cfg } = params; const commandSessionKey = normalizeOptionalString(ctx.SessionKey) ?? normalizeOptionalString(ctx.ParentSessionKey); @@ -401,20 +434,26 @@ export async function tryFastAbortFromMessage(params: { ]), ); let aborted = false; + let activeAbortRejected = false; for (const abortTargetKey of abortTargetKeys) { - aborted = - abortSessionRunTarget({ - key: abortTargetKey, - sessionId: sessionIdsByKey.get(abortTargetKey), - }) || aborted; + const outcome = abortSessionRunTargetWithOutcome({ + key: abortTargetKey, + sessionId: sessionIdsByKey.get(abortTargetKey), + }); + activeAbortRejected ||= outcome.active && !outcome.aborted; + aborted = outcome.aborted || aborted; } const sourceSessionId = sourceAbortKey ? (replyRunRegistry.resolveSessionId(sourceAbortKey) ?? resolveStoredSessionId({ cfg, sessionKey: sourceAbortKey })) : undefined; if (sourceAbortKey) { - aborted = - abortSessionRunTarget({ key: sourceAbortKey, sessionId: sourceSessionId }) || aborted; + const outcome = abortSessionRunTargetWithOutcome({ + key: sourceAbortKey, + sessionId: sourceSessionId, + }); + activeAbortRejected ||= outcome.active && !outcome.aborted; + aborted = outcome.aborted || aborted; } const cleared = clearSessionQueues( abortTargetKeys @@ -427,6 +466,14 @@ export async function tryFastAbortFromMessage(params: { ); } const { stopped } = stopSubagentsForRequester({ cfg, requesterSessionKey }); + if (activeAbortRejected && !aborted) { + return { + handled: true, + aborted: false, + rejectionReason: "finalizing", + stoppedSubagents: stopped, + }; + } let persistedAbortTarget: SessionAbortTargetResult | null = null; try { persistedAbortTarget = await abortDeps.markSessionAbortTarget({ diff --git a/src/auto-reply/reply/agent-runner-direct-runtime-config.test.ts b/src/auto-reply/reply/agent-runner-direct-runtime-config.test.ts index 4d503c6984b5..98bbcd6c2aff 100644 --- a/src/auto-reply/reply/agent-runner-direct-runtime-config.test.ts +++ b/src/auto-reply/reply/agent-runner-direct-runtime-config.test.ts @@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { getReplyPayloadMetadata } from "../reply-payload.js"; import type { TemplateContext } from "../templating.js"; +import { SILENT_REPLY_TOKEN } from "../tokens.js"; import { createTestFollowupRun } from "./agent-runner.test-fixtures.js"; import type { QueueSettings } from "./queue.js"; import { createMockTypingController } from "./test-helpers.js"; @@ -251,12 +252,17 @@ describe("runReplyAgent runtime config", () => { shouldFollowup: false, isActive: false, }); + let observedReplyOperation: { freezeAbort: () => void } | undefined; replyParams.opts = { sourceReplyDeliveryMode: "message_tool_only" }; runPreflightCompactionIfNeededMock.mockResolvedValue(undefined); runMemoryFlushIfNeededMock.mockImplementation( async (params: { + replyOperation: { freezeAbort: () => void }; onVisibleErrorPayloads?: (payloads: Array<{ text?: string; isError?: boolean }>) => void; }) => { + vi.spyOn(params.replyOperation, "freezeAbort"); + observedReplyOperation = params.replyOperation; + expect(params.replyOperation.freezeAbort).not.toHaveBeenCalled(); params.onVisibleErrorPayloads?.([ { text: "⚠️ write failed: Memory flush writes are restricted to memory/2023-11-14.md; use that path only.", @@ -269,6 +275,10 @@ describe("runReplyAgent runtime config", () => { const result = await runReplyAgent(replyParams); + if (!observedReplyOperation) { + throw new Error("expected memory flush reply operation"); + } + expect(observedReplyOperation.freezeAbort).toHaveBeenCalledTimes(1); if (!result || Array.isArray(result)) { throw new Error("expected a single memory-flush error reply payload"); } @@ -295,6 +305,24 @@ describe("runReplyAgent runtime config", () => { }); }); + it("does not start the main turn after cancellation during memory flush", async () => { + const { replyParams } = createDirectRuntimeReplyParams({ + shouldFollowup: false, + isActive: false, + }); + runPreflightCompactionIfNeededMock.mockResolvedValue(undefined); + runMemoryFlushIfNeededMock.mockImplementation( + async (params: { replyOperation: { abortByUser: () => boolean } }) => { + expect(params.replyOperation.abortByUser()).toBe(true); + return undefined; + }, + ); + + const result = await runReplyAgent(replyParams); + + expect(result).toMatchObject({ text: SILENT_REPLY_TOKEN }); + }); + it("surfaces known pre-run Codex usage-limit failures instead of dropping the reply", async () => { const { replyParams } = createDirectRuntimeReplyParams({ shouldFollowup: false, diff --git a/src/auto-reply/reply/agent-runner-execution.test.ts b/src/auto-reply/reply/agent-runner-execution.test.ts index b4fee847e59d..bdbeb672cd84 100644 --- a/src/auto-reply/reply/agent-runner-execution.test.ts +++ b/src/auto-reply/reply/agent-runner-execution.test.ts @@ -6,6 +6,7 @@ import { formatBillingErrorMessage } from "../../agents/embedded-agent-helpers.j import { FailoverError } from "../../agents/failover-error.js"; import { LiveSessionModelSwitchError } from "../../agents/live-model-switch-error.js"; import { MissingProviderAuthError } from "../../agents/model-auth.js"; +import { createAgentRunRestartAbortError } from "../../agents/run-termination.js"; import type { SessionEntry } from "../../config/sessions.js"; import type { ModelDefinitionConfig } from "../../config/types.models.js"; import { @@ -38,7 +39,7 @@ import { PROVIDER_RATE_LIMIT_OR_QUOTA_ERROR_USER_MESSAGE, } from "./provider-request-error-classifier.js"; import type { FollowupRun } from "./queue.js"; -import type { ReplyOperation } from "./reply-run-registry.js"; +import { createReplyOperation, type ReplyOperation } from "./reply-run-registry.js"; import type { TypingSignaler } from "./typing-mode.js"; const state = vi.hoisted(() => ({ @@ -443,14 +444,17 @@ function createTestUserTurnRecorder(message: PersistedUserTurnMessage) { function createMockReplyOperation(): { replyOperation: ReplyOperation; failMock: ReturnType; + freezeAbortMock: ReturnType; retainFailureUntilCompleteMock: ReturnType; updateSessionIdMock: ReturnType; } { const failMock = vi.fn(); + const freezeAbortMock = vi.fn(); const retainFailureUntilCompleteMock = vi.fn(); const updateSessionIdMock = vi.fn(); return { failMock, + freezeAbortMock, retainFailureUntilCompleteMock, updateSessionIdMock, replyOperation: { @@ -465,13 +469,14 @@ function createMockReplyOperation(): { updateSessionId: updateSessionIdMock, attachBackend: vi.fn(), detachBackend: vi.fn(), + freezeAbort: freezeAbortMock, retainFailureUntilComplete: retainFailureUntilCompleteMock, complete: vi.fn(), completeThen: vi.fn((afterClear: () => void) => afterClear()), completeWithAfterClearBarrier: vi.fn(), fail: failMock, - abortByUser: vi.fn(), - abortForRestart: vi.fn(), + abortByUser: vi.fn(() => true), + abortForRestart: vi.fn(() => true), markTerminalRecovery: vi.fn(), }, }; @@ -1337,6 +1342,220 @@ describe("runAgentTurnWithFallback", () => { expect(embeddedCall.abortSignal).toBe(replyOperation.abortSignal); }); + it("freezes abort ownership only after model fallback settles", async () => { + const { replyOperation, freezeAbortMock } = createMockReplyOperation(); + state.runWithModelFallbackMock.mockImplementationOnce(async (params: FallbackRunnerParams) => { + expect(freezeAbortMock).not.toHaveBeenCalled(); + await params.run("anthropic", "claude").catch(() => undefined); + expect(freezeAbortMock).not.toHaveBeenCalled(); + const result = await params.run("openai", "gpt-5.5"); + expect(freezeAbortMock).not.toHaveBeenCalled(); + return { + result, + provider: "openai", + model: "gpt-5.5", + attempts: [], + }; + }); + state.runEmbeddedAgentMock + .mockRejectedValueOnce(new Error("primary failed")) + .mockResolvedValueOnce({ + payloads: [{ text: "ok" }], + meta: {}, + }); + + const runAgentTurnWithFallback = await getRunAgentTurnWithFallback(); + await runAgentTurnWithFallback({ + ...createMinimalRunAgentTurnParams(), + replyOperation, + }); + + expect(state.runEmbeddedAgentMock).toHaveBeenCalledTimes(2); + expect(freezeAbortMock).toHaveBeenCalledTimes(1); + }); + + it("suppresses a settled fallback result after an accepted user abort", async () => { + const { replyOperation, freezeAbortMock } = createMockReplyOperation(); + const abortController = new AbortController(); + let operationResult: ReplyOperation["result"] = null; + let releaseFallback: () => void = () => undefined; + let markCandidateSettled: () => void = () => undefined; + const candidateSettled = new Promise((resolve) => { + markCandidateSettled = resolve; + }); + const fallbackRelease = new Promise((resolve) => { + releaseFallback = resolve; + }); + let releaseToolTask: () => void = () => undefined; + const pendingToolTask = new Promise((resolve) => { + releaseToolTask = resolve; + }); + const pendingToolTasks = new Set([pendingToolTask]); + Object.defineProperty(replyOperation, "abortSignal", { + configurable: true, + get: () => abortController.signal, + }); + Object.defineProperty(replyOperation, "result", { + configurable: true, + get: () => operationResult, + }); + replyOperation.abortByUser = vi.fn(() => { + operationResult = { kind: "aborted", code: "aborted_by_user" }; + abortController.abort("user_abort"); + return true; + }); + state.runEmbeddedAgentMock.mockResolvedValueOnce({ + payloads: [{ text: "late reply" }], + meta: {}, + }); + state.runWithModelFallbackMock.mockImplementationOnce(async (params: FallbackRunnerParams) => { + const result = await params.run("anthropic", "claude"); + markCandidateSettled(); + await fallbackRelease; + return { + result, + provider: "anthropic", + model: "claude", + attempts: [], + }; + }); + + const runAgentTurnWithFallback = await getRunAgentTurnWithFallback(); + const pending = runAgentTurnWithFallback({ + ...createMinimalRunAgentTurnParams(), + replyOperation, + pendingToolTasks, + }); + await candidateSettled; + expect(replyOperation.abortByUser()).toBe(true); + let settled = false; + void pending.then(() => { + settled = true; + }); + releaseFallback(); + await new Promise((resolve) => { + setImmediate(resolve); + }); + expect(settled).toBe(false); + expect(freezeAbortMock).not.toHaveBeenCalled(); + releaseToolTask(); + + await expect(pending).resolves.toEqual({ + kind: "final", + payload: { text: SILENT_REPLY_TOKEN }, + }); + expect(freezeAbortMock).toHaveBeenCalledTimes(1); + }); + + it("suppresses a settled fallback result after an upstream abort", async () => { + const upstreamAbort = new AbortController(); + const replyOperation = createReplyOperation({ + sessionKey: "agent:main:upstream-settled-fallback", + sessionId: "upstream-settled-fallback", + resetTriggered: false, + upstreamAbortSignal: upstreamAbort.signal, + }); + replyOperation.setPhase("running"); + let releaseFallback: () => void = () => undefined; + let markCandidateSettled: () => void = () => undefined; + const candidateSettled = new Promise((resolve) => { + markCandidateSettled = resolve; + }); + const fallbackRelease = new Promise((resolve) => { + releaseFallback = resolve; + }); + state.runEmbeddedAgentMock.mockResolvedValueOnce({ + payloads: [{ text: "late reply" }], + meta: {}, + }); + state.runWithModelFallbackMock.mockImplementationOnce(async (params: FallbackRunnerParams) => { + const result = await params.run("anthropic", "claude"); + markCandidateSettled(); + await fallbackRelease; + return { + result, + provider: "anthropic", + model: "claude", + attempts: [], + }; + }); + + try { + const runAgentTurnWithFallback = await getRunAgentTurnWithFallback(); + const pending = runAgentTurnWithFallback({ + ...createMinimalRunAgentTurnParams(), + replyOperation, + }); + await candidateSettled; + upstreamAbort.abort(new Error("caller cancelled")); + expect(replyOperation.result).toEqual({ kind: "aborted", code: "aborted_by_user" }); + expect(replyOperation.abortSignal.aborted).toBe(true); + releaseFallback(); + + await expect(pending).resolves.toEqual({ + kind: "final", + payload: { text: SILENT_REPLY_TOKEN }, + }); + } finally { + replyOperation.complete(); + } + }); + + it("preserves restart reply classification after an upstream abort settles fallback", async () => { + const upstreamAbort = new AbortController(); + const replyOperation = createReplyOperation({ + sessionKey: "agent:main:upstream-settled-restart", + sessionId: "upstream-settled-restart", + resetTriggered: false, + upstreamAbortSignal: upstreamAbort.signal, + }); + replyOperation.setPhase("running"); + let releaseFallback: () => void = () => undefined; + let markCandidateSettled: () => void = () => undefined; + const candidateSettled = new Promise((resolve) => { + markCandidateSettled = resolve; + }); + const fallbackRelease = new Promise((resolve) => { + releaseFallback = resolve; + }); + state.runEmbeddedAgentMock.mockResolvedValueOnce({ + payloads: [{ text: "late reply" }], + meta: {}, + }); + state.runWithModelFallbackMock.mockImplementationOnce(async (params: FallbackRunnerParams) => { + const result = await params.run("anthropic", "claude"); + markCandidateSettled(); + await fallbackRelease; + return { + result, + provider: "anthropic", + model: "claude", + attempts: [], + }; + }); + + try { + const runAgentTurnWithFallback = await getRunAgentTurnWithFallback(); + const pending = runAgentTurnWithFallback({ + ...createMinimalRunAgentTurnParams(), + replyOperation, + }); + await candidateSettled; + upstreamAbort.abort(createAgentRunRestartAbortError()); + releaseFallback(); + + await expect(pending).resolves.toEqual({ + kind: "final", + payload: { + isError: true, + text: "⚠️ Gateway is restarting. Please wait a few seconds and try again.", + }, + }); + } finally { + replyOperation.complete(); + } + }); + it("passes the hydrated run account to embedded execution", async () => { state.runEmbeddedAgentMock.mockResolvedValueOnce({ payloads: [{ text: "ok" }], diff --git a/src/auto-reply/reply/agent-runner-execution.ts b/src/auto-reply/reply/agent-runner-execution.ts index 0e4dec9a752e..6eb83cd171df 100644 --- a/src/auto-reply/reply/agent-runner-execution.ts +++ b/src/auto-reply/reply/agent-runner-execution.ts @@ -141,6 +141,7 @@ import { import { resolveCurrentTurnImages } from "./current-turn-images.js"; import { hasInboundAudio } from "./inbound-media.js"; import { resolveOriginMessageProvider } from "./origin-routing.js"; +import { drainPendingToolTasks } from "./pending-tool-task-drain.js"; import { classifyProviderRequestError, PROVIDER_CONVERSATION_STATE_ERROR_USER_MESSAGE, @@ -1475,16 +1476,25 @@ function resolveRestartLifecycleError( } function isReplyOperationUserAbort(replyOperation?: ReplyOperation): boolean { - return ( - replyOperation?.result?.kind === "aborted" && replyOperation.result.code === "aborted_by_user" - ); + if ( + replyOperation?.result?.kind === "aborted" && + replyOperation.result.code === "aborted_by_user" + ) { + return true; + } + const abortSignal = replyOperation?.abortSignal; + return abortSignal?.aborted === true && !isAgentRunRestartAbortReason(abortSignal.reason); } function isReplyOperationRestartAbort(replyOperation?: ReplyOperation): boolean { - return ( + if ( replyOperation?.result?.kind === "aborted" && replyOperation.result.code === "aborted_for_restart" - ); + ) { + return true; + } + const abortSignal = replyOperation?.abortSignal; + return abortSignal?.aborted === true && isAgentRunRestartAbortReason(abortSignal.reason); } function emitModelFallbackStepLifecycle(params: { @@ -1593,42 +1603,44 @@ export function resolveRunAfterAutoFallbackPrimaryProbeRecheck(params: { }; } -/** Runs the agent turn with provider/model fallback, retry, and failure mapping. */ -export async function runAgentTurnWithFallback(params: { - commandBody: string; - transcriptCommandBody?: string; - followupRun: FollowupRun; - sessionCtx: TemplateContext; - replyThreading?: TemplateContext["ReplyThreading"]; - replyOperation?: ReplyOperation; - opts?: GetReplyOptions; - typingSignals: TypingSignaler; - blockReplyPipeline: BlockReplyPipeline | null; - blockStreamingEnabled: boolean; - blockReplyChunking?: { - minChars: number; - maxChars: number; - breakPreference: "paragraph" | "newline" | "sentence"; - flushOnParagraph?: boolean; - }; - resolvedBlockStreamingBreak: "text_end" | "message_end"; - applyReplyToMode: (payload: ReplyPayload) => ReplyPayload; - shouldEmitToolResult: () => boolean; - shouldEmitToolOutput: () => boolean; - pendingToolTasks: Set>; - resetSessionAfterRoleOrderingConflict: (reason: string) => Promise; - isHeartbeat: boolean; - sessionKey?: string; - runtimePolicySessionKey?: string; - getActiveSessionEntry: () => SessionEntry | undefined; - activeSessionStore?: Record; - storePath?: string; - resolvedVerboseLevel: VerboseLevel; - toolProgressDetail?: "explain" | "raw"; - replyMediaContext?: ReplyMediaContext; - onCompactionNoticePayload?: (payload: ReplyPayload) => Promise | void; - isRestartRecoveryArmed?: () => boolean; -}): Promise { +async function runAgentTurnWithFallbackInternal( + params: { + commandBody: string; + transcriptCommandBody?: string; + followupRun: FollowupRun; + sessionCtx: TemplateContext; + replyThreading?: TemplateContext["ReplyThreading"]; + replyOperation?: ReplyOperation; + opts?: GetReplyOptions; + typingSignals: TypingSignaler; + blockReplyPipeline: BlockReplyPipeline | null; + blockStreamingEnabled: boolean; + blockReplyChunking?: { + minChars: number; + maxChars: number; + breakPreference: "paragraph" | "newline" | "sentence"; + flushOnParagraph?: boolean; + }; + resolvedBlockStreamingBreak: "text_end" | "message_end"; + applyReplyToMode: (payload: ReplyPayload) => ReplyPayload; + shouldEmitToolResult: () => boolean; + shouldEmitToolOutput: () => boolean; + pendingToolTasks: Set>; + resetSessionAfterRoleOrderingConflict: (reason: string) => Promise; + isHeartbeat: boolean; + sessionKey?: string; + runtimePolicySessionKey?: string; + getActiveSessionEntry: () => SessionEntry | undefined; + activeSessionStore?: Record; + storePath?: string; + resolvedVerboseLevel: VerboseLevel; + toolProgressDetail?: "explain" | "raw"; + replyMediaContext?: ReplyMediaContext; + onCompactionNoticePayload?: (payload: ReplyPayload) => Promise | void; + isRestartRecoveryArmed?: () => boolean; + }, + commitTerminalOutcome: () => void, +): Promise { const TRANSIENT_HTTP_RETRY_DELAY_MS = 2_500; let didLogHeartbeatStrip = false; let autoCompactionCount = 0; @@ -3046,6 +3058,20 @@ export async function runAgentTurnWithFallback(params: { ? restartAbortReason : createAgentRunRestartAbortError(); } + if (isReplyOperationUserAbort(params.replyOperation)) { + settledLifecycleTerminal?.emit("end", runResult); + await drainPendingToolTasks({ + tasks: params.pendingToolTasks, + onTimeout: logVerbose, + }); + return { + kind: "final", + payload: { + text: SILENT_REPLY_TOKEN, + }, + }; + } + commitTerminalOutcome(); fallbackAttempts = Array.isArray(fallbackResult.attempts) ? fallbackResult.attempts.map((attempt) => ({ provider: attempt.provider, @@ -3512,3 +3538,22 @@ export async function runAgentTurnWithFallback(params: { ), }; } + +/** Runs the agent turn with provider/model fallback, retry, and failure mapping. */ +export async function runAgentTurnWithFallback( + params: Parameters[0], +): Promise { + let terminalOutcomeCommitted = false; + const commitTerminalOutcome = () => { + if (terminalOutcomeCommitted) { + return; + } + terminalOutcomeCommitted = true; + params.replyOperation?.freezeAbort(); + }; + try { + return await runAgentTurnWithFallbackInternal(params, commitTerminalOutcome); + } finally { + commitTerminalOutcome(); + } +} diff --git a/src/auto-reply/reply/agent-runner-memory.preflight-stale-tokens.test.ts b/src/auto-reply/reply/agent-runner-memory.preflight-stale-tokens.test.ts index 9d3467845433..d551104b0f66 100644 --- a/src/auto-reply/reply/agent-runner-memory.preflight-stale-tokens.test.ts +++ b/src/auto-reply/reply/agent-runner-memory.preflight-stale-tokens.test.ts @@ -30,6 +30,7 @@ function createReplyOperation(): ReplyOperation { updateSessionId: vi.fn(), attachBackend: vi.fn(), detachBackend: vi.fn(), + freezeAbort: vi.fn(), retainFailureUntilComplete: vi.fn(), complete: vi.fn(), completeThen: vi.fn((afterClear: () => void) => { @@ -113,7 +114,11 @@ describe("runPreflightCompactionIfNeeded stale totalTokens gating", () => { totalTokens: 200_000, totalTokensFresh: false, }; - await writeTestSessionStore(path.join(rootDir, "sessions.json"), "agent:main:main", sessionEntry); + await writeTestSessionStore( + path.join(rootDir, "sessions.json"), + "agent:main:main", + sessionEntry, + ); const entry = await runWithEntry(sessionEntry, sessionFile); @@ -135,7 +140,11 @@ describe("runPreflightCompactionIfNeeded stale totalTokens gating", () => { totalTokens: 200_000, totalTokensFresh: true, }; - await writeTestSessionStore(path.join(rootDir, "sessions.json"), "agent:main:main", sessionEntry); + await writeTestSessionStore( + path.join(rootDir, "sessions.json"), + "agent:main:main", + sessionEntry, + ); await runWithEntry(sessionEntry, sessionFile); diff --git a/src/auto-reply/reply/agent-runner-memory.test.ts b/src/auto-reply/reply/agent-runner-memory.test.ts index 72ed1c495f80..7b26dfdf74b0 100644 --- a/src/auto-reply/reply/agent-runner-memory.test.ts +++ b/src/auto-reply/reply/agent-runner-memory.test.ts @@ -53,6 +53,7 @@ function createReplyOperation(): TestReplyOperation { updateSessionId: vi.fn(), attachBackend: vi.fn(), detachBackend: vi.fn(), + freezeAbort: vi.fn(), retainFailureUntilComplete: vi.fn(), complete: vi.fn(), completeThen: vi.fn((afterClear: () => void) => { @@ -60,8 +61,8 @@ function createReplyOperation(): TestReplyOperation { }), completeWithAfterClearBarrier: vi.fn(), fail: vi.fn(), - abortByUser: vi.fn(), - abortForRestart: vi.fn(), + abortByUser: vi.fn(() => true), + abortForRestart: vi.fn(() => true), markTerminalRecovery: vi.fn(), }; } @@ -102,6 +103,7 @@ type EmbeddedAgentParams = { bootstrapPromptWarningSignaturesSeen?: string[]; bootstrapPromptWarningSignature?: string; abortSignal?: AbortSignal; + isFinalFallbackAttempt?: boolean; }; type CompactEmbeddedAgentSessionParams = { @@ -816,6 +818,24 @@ describe("runMemoryFlushIfNeeded", () => { agentRuntimeOverride: "codex", }; const runtimePolicySessionKey = "agent:main:telegram:default:direct:12345"; + runWithModelFallbackMock.mockImplementationOnce( + async (params: { + provider: string; + model: string; + run: ( + provider: string, + model: string, + options?: { isFinalFallbackAttempt?: boolean }, + ) => Promise; + }) => ({ + result: await params.run(params.provider, params.model, { + isFinalFallbackAttempt: false, + }), + provider: params.provider, + model: params.model, + attempts: [], + }), + ); await runMemoryFlushIfNeeded({ cfg, @@ -843,6 +863,7 @@ describe("runMemoryFlushIfNeeded", () => { expect(fallbackCall.agentId).toBe("main"); expect(fallbackCall.sessionKey).toBe(runtimePolicySessionKey); expect(fallbackCall.resolveAgentHarnessRuntimeOverride?.("openai", "gpt-5.4")).toBe("codex"); + expect(requireEmbeddedAgentCall().isFinalFallbackAttempt).toBe(false); await fallbackCall.prepareAgentHarnessRuntime?.({ provider: "openai", diff --git a/src/auto-reply/reply/agent-runner-memory.ts b/src/auto-reply/reply/agent-runner-memory.ts index d22a5ab84ce9..0138f3f05fab 100644 --- a/src/auto-reply/reply/agent-runner-memory.ts +++ b/src/auto-reply/reply/agent-runner-memory.ts @@ -1358,6 +1358,7 @@ export async function runMemoryFlushIfNeeded(params: { prompt: activeMemoryFlushPlan.prompt, transcriptPrompt: "", extraSystemPrompt: flushSystemPrompt, + isFinalFallbackAttempt: runOptions?.isFinalFallbackAttempt, bootstrapPromptWarningSignaturesSeen, bootstrapPromptWarningSignature: bootstrapPromptWarningSignaturesSeen[bootstrapPromptWarningSignaturesSeen.length - 1], diff --git a/src/auto-reply/reply/agent-runner.final-media-runreplyagent.test.ts b/src/auto-reply/reply/agent-runner.final-media-runreplyagent.test.ts index b66404b9b78b..6e2b6c6e16b1 100644 --- a/src/auto-reply/reply/agent-runner.final-media-runreplyagent.test.ts +++ b/src/auto-reply/reply/agent-runner.final-media-runreplyagent.test.ts @@ -103,6 +103,7 @@ function createReplyOperation(): ReplyOperation { return { result: undefined, setPhase: vi.fn(), + freezeAbort: vi.fn(), fail: vi.fn(), complete: vi.fn(), completeThen: vi.fn(), diff --git a/src/auto-reply/reply/agent-runner.media-paths.test.ts b/src/auto-reply/reply/agent-runner.media-paths.test.ts index 1188649824f5..290ed9853dd7 100644 --- a/src/auto-reply/reply/agent-runner.media-paths.test.ts +++ b/src/auto-reply/reply/agent-runner.media-paths.test.ts @@ -260,6 +260,7 @@ function createReplyOperation(): ReplyOperation { return { result: undefined, setPhase: vi.fn(), + freezeAbort: vi.fn(), fail: vi.fn(), complete: vi.fn(), completeThen: vi.fn(), diff --git a/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts b/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts index 29677155bbf6..af76cf473c82 100644 --- a/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts +++ b/src/auto-reply/reply/agent-runner.misc.runreplyagent.test.ts @@ -27,6 +27,7 @@ import { } from "../../plugins/memory-state.js"; import { GatewayDrainingError } from "../../process/command-queue.js"; import type { TemplateContext } from "../templating.js"; +import { SILENT_REPLY_TOKEN } from "../tokens.js"; import type { FollowupRun, QueueSettings } from "./queue.js"; import { scheduleFollowupDrain } from "./queue.js"; import { @@ -605,6 +606,82 @@ describe("runReplyAgent auto-compaction token update", () => { expect(scheduleFollowupDrain).toHaveBeenCalledTimes(1); }); + it("records a settled fallback cancelled by its upstream signal as aborted", async () => { + const upstreamAbort = new AbortController(); + const sessionKey = "upstream-cancelled-settled-fallback"; + const sessionEntry = { + sessionId: "session-upstream-cancelled", + updatedAt: Date.now(), + totalTokens: 50_000, + }; + const replyOperation = createReplyOperation({ + sessionKey, + sessionId: sessionEntry.sessionId, + resetTriggered: false, + upstreamAbortSignal: upstreamAbort.signal, + }); + let releaseFallback: () => void = () => undefined; + let markCandidateSettled: () => void = () => undefined; + const candidateSettled = new Promise((resolve) => { + markCandidateSettled = resolve; + }); + const fallbackRelease = new Promise((resolve) => { + releaseFallback = resolve; + }); + runEmbeddedAgentMock.mockResolvedValueOnce({ + payloads: [{ text: "late reply" }], + meta: { agentMeta: {} }, + }); + runWithModelFallbackMock.mockImplementationOnce( + async ({ provider, model, run }: RunWithModelFallbackParams) => { + const result = await run(provider, model); + markCandidateSettled(); + await fallbackRelease; + return { result, provider, model }; + }, + ); + const { typing, sessionCtx, resolvedQueue, followupRun } = createBaseRun({ + storePath: "", + sessionEntry, + }); + followupRun.run.sessionKey = sessionKey; + + try { + const pending = runReplyAgent({ + commandBody: "hello", + followupRun, + queueKey: sessionKey, + resolvedQueue, + shouldSteer: false, + shouldFollowup: false, + isActive: false, + isStreaming: false, + typing, + sessionCtx, + sessionEntry, + sessionStore: { [sessionKey]: sessionEntry }, + sessionKey, + defaultModel: "anthropic/claude-opus-4-6", + agentCfgContextTokens: 200_000, + resolvedVerboseLevel: "off", + isNewSession: false, + blockStreamingEnabled: false, + resolvedBlockStreamingBreak: "message_end", + shouldInjectGroupIntro: false, + typingMode: "instant", + replyOperation, + }); + await candidateSettled; + upstreamAbort.abort(new Error("caller cancelled")); + releaseFallback(); + + expectReplyText(await pending, SILENT_REPLY_TOKEN); + expect(replyOperation.result).toEqual({ kind: "aborted", code: "aborted_by_user" }); + } finally { + replyOperation.complete(); + } + }); + it("reports live diagnostic context from promptTokens, not provider usage totals", async () => { const { usageEvent } = await runBaseReplyWithAgentMeta({ tmpPrefix: "openclaw-usage-diagnostic-", diff --git a/src/auto-reply/reply/agent-runner.ts b/src/auto-reply/reply/agent-runner.ts index a6ee1a8f4074..d6003bcf8dc4 100644 --- a/src/auto-reply/reply/agent-runner.ts +++ b/src/auto-reply/reply/agent-runner.ts @@ -1624,6 +1624,10 @@ export async function runReplyAgent(params: { }), ); + if (replyOperation.result?.kind === "aborted") { + throw replyOperation.abortSignal.reason ?? new Error("reply operation aborted"); + } + if (visibleMemoryFlushErrorPayloads.length > 0) { const currentMessageId = sessionCtx.MessageSidFull ?? sessionCtx.MessageSid; const payloadResult = await buildReplyPayloads({ @@ -1653,6 +1657,7 @@ export async function runReplyAgent(params: { markReplyPayloadForSourceSuppressionDelivery(payload), ); if (replyPayloads.length > 0) { + replyOperation.freezeAbort(); replyOperation.fail( "run_failed", new Error("memory flush produced visible error payloads"), diff --git a/src/auto-reply/reply/commands-abort-trigger.test.ts b/src/auto-reply/reply/commands-abort-trigger.test.ts index d3cfeec1b594..84c2dede0eb1 100644 --- a/src/auto-reply/reply/commands-abort-trigger.test.ts +++ b/src/auto-reply/reply/commands-abort-trigger.test.ts @@ -11,7 +11,10 @@ const resolveCommandSessionEntryForKeyMock = vi.hoisted(() => vi.fn(() => ({ entry: undefined, key: "agent:main:main" })), ); const setAbortMemoryMock = vi.hoisted(() => vi.fn()); -const abortSessionRunTargetMock = vi.hoisted(() => vi.fn()); +const abortSessionRunTargetWithOutcomeMock = vi.hoisted(() => + vi.fn(() => ({ active: false, aborted: false })), +); +const formatAbortReplyTextMock = vi.hoisted(() => vi.fn(() => "⚙️ Agent was aborted.")); vi.mock("../../agents/embedded-agent.js", () => ({ abortEmbeddedAgentRun: abortEmbeddedAgentRunMock, @@ -32,8 +35,8 @@ vi.mock("./abort-cutoff.js", () => ({ })); vi.mock("./abort.js", () => ({ - abortSessionRunTarget: abortSessionRunTargetMock, - formatAbortReplyText: vi.fn(() => "⚙️ Agent was aborted."), + abortSessionRunTargetWithOutcome: abortSessionRunTargetWithOutcomeMock, + formatAbortReplyText: formatAbortReplyTextMock, isAbortTrigger: vi.fn((raw: string) => raw === "stop"), setAbortMemory: setAbortMemoryMock, stopSubagentsForRequester: vi.fn(() => ({ stopped: 0 })), @@ -94,14 +97,35 @@ function buildAbortParams(): HandleCommandsParams { describe("handleAbortTrigger", () => { beforeEach(() => { vi.clearAllMocks(); + abortSessionRunTargetWithOutcomeMock.mockReturnValue({ active: false, aborted: false }); }); it("rejects unauthorized natural-language abort triggers", async () => { const result = await handleAbortTrigger(buildAbortParams(), true); expect(result).toEqual({ shouldContinue: false }); - expect(abortSessionRunTargetMock).not.toHaveBeenCalled(); + expect(abortSessionRunTargetWithOutcomeMock).not.toHaveBeenCalled(); expect(abortEmbeddedAgentRunMock).not.toHaveBeenCalled(); expect(persistAbortTargetEntryMock).not.toHaveBeenCalled(); expect(setAbortMemoryMock).not.toHaveBeenCalled(); }); + + it("reports a finalizing run without persisting abort state", async () => { + const params = buildAbortParams(); + params.command.isAuthorizedSender = true; + params.command.senderIsOwner = true; + abortSessionRunTargetWithOutcomeMock.mockReturnValue({ active: true, aborted: false }); + formatAbortReplyTextMock.mockReturnValue( + "Agent reply is already finalizing and can no longer be aborted.", + ); + + const result = await handleAbortTrigger(params, true); + + expect(result).toEqual({ + shouldContinue: false, + reply: { text: "Agent reply is already finalizing and can no longer be aborted." }, + }); + expect(formatAbortReplyTextMock).toHaveBeenCalledWith(undefined, "finalizing"); + expect(persistAbortTargetEntryMock).not.toHaveBeenCalled(); + expect(setAbortMemoryMock).not.toHaveBeenCalled(); + }); }); diff --git a/src/auto-reply/reply/commands-compact.test.ts b/src/auto-reply/reply/commands-compact.test.ts index f0d18c16d249..77204ea964ea 100644 --- a/src/auto-reply/reply/commands-compact.test.ts +++ b/src/auto-reply/reply/commands-compact.test.ts @@ -226,8 +226,9 @@ describe("handleCompactCommand", () => { expect(vi.mocked(compactEmbeddedAgentSession)).toHaveBeenCalledOnce(); }); - it("aborts an active embedded run before compacting", async () => { + it("waits for an active embedded run before compacting even when abort is rejected", async () => { vi.mocked(isEmbeddedAgentRunAbortableForCompaction).mockReturnValueOnce(true); + vi.mocked(abortEmbeddedAgentRun).mockReturnValueOnce(false); vi.mocked(compactEmbeddedAgentSession).mockResolvedValueOnce({ ok: true, compacted: false, diff --git a/src/auto-reply/reply/commands-session-abort.ts b/src/auto-reply/reply/commands-session-abort.ts index b0b15d7aece2..8ad155216ede 100644 --- a/src/auto-reply/reply/commands-session-abort.ts +++ b/src/auto-reply/reply/commands-session-abort.ts @@ -9,7 +9,7 @@ import { type AbortCutoff, } from "./abort-cutoff.js"; import { - abortSessionRunTarget, + abortSessionRunTargetWithOutcome, formatAbortReplyText, isAbortTrigger, setAbortMemory, @@ -89,7 +89,13 @@ async function applyAbortTarget(params: { abortCutoff?: AbortCutoff; }) { const { abortTarget } = params; - abortSessionRunTarget({ key: abortTarget.key, sessionId: abortTarget.sessionId }); + const abortOutcome = abortSessionRunTargetWithOutcome({ + key: abortTarget.key, + sessionId: abortTarget.sessionId, + }); + if (abortOutcome.active && !abortOutcome.aborted) { + return abortOutcome; + } const persisted = await persistAbortTargetEntry({ entry: abortTarget.entry, @@ -101,6 +107,7 @@ async function applyAbortTarget(params: { if (!persisted && params.abortKey) { setAbortMemory(params.abortKey, true); } + return abortOutcome; } function buildAbortTargetApplyParams( @@ -143,7 +150,7 @@ export const handleStopCommand: CommandHandler = async (params, allowTextCommand `stop: cleared followups=${cleared.followupCleared} lane=${cleared.laneCleared} keys=${cleared.keys.join(",")}`, ); } - await applyAbortTarget(buildAbortTargetApplyParams(params, abortTarget)); + const abortOutcome = await applyAbortTarget(buildAbortTargetApplyParams(params, abortTarget)); // Trigger internal hook for stop command const hookEvent = createInternalHookEvent( @@ -164,7 +171,12 @@ export const handleStopCommand: CommandHandler = async (params, allowTextCommand requesterSessionKey: abortTarget.key ?? params.sessionKey, }); - return { shouldContinue: false, reply: { text: formatAbortReplyText(stopped) } }; + const rejectionReason = + abortOutcome.active && !abortOutcome.aborted ? ("finalizing" as const) : undefined; + return { + shouldContinue: false, + reply: { text: formatAbortReplyText(stopped, rejectionReason) }, + }; }; export const handleAbortTrigger: CommandHandler = async (params, allowTextCommands) => { @@ -184,6 +196,11 @@ export const handleAbortTrigger: CommandHandler = async (params, allowTextComman sessionEntry: params.sessionEntry, sessionStore: params.sessionStore, }); - await applyAbortTarget(buildAbortTargetApplyParams(params, abortTarget)); - return { shouldContinue: false, reply: { text: "⚙️ Agent was aborted." } }; + const abortOutcome = await applyAbortTarget(buildAbortTargetApplyParams(params, abortTarget)); + const rejectionReason = + abortOutcome.active && !abortOutcome.aborted ? ("finalizing" as const) : undefined; + return { + shouldContinue: false, + reply: { text: formatAbortReplyText(undefined, rejectionReason) }, + }; }; diff --git a/src/auto-reply/reply/commands-stop-target.test.ts b/src/auto-reply/reply/commands-stop-target.test.ts index f9fc67ee253c..0e46acf86a06 100644 --- a/src/auto-reply/reply/commands-stop-target.test.ts +++ b/src/auto-reply/reply/commands-stop-target.test.ts @@ -21,7 +21,10 @@ const resolveCommandSessionEntryForKeyMock = vi.hoisted(() => ); const resolveSessionIdMock = vi.hoisted(() => vi.fn(() => undefined)); const stopSubagentsForRequesterMock = vi.hoisted(() => vi.fn(() => ({ stopped: 0 }))); -const abortSessionRunTargetMock = vi.hoisted(() => vi.fn()); +const abortSessionRunTargetWithOutcomeMock = vi.hoisted(() => + vi.fn(() => ({ active: false, aborted: false })), +); +const formatAbortReplyTextMock = vi.hoisted(() => vi.fn(() => "⚙️ Agent was aborted.")); vi.mock("../../agents/embedded-agent.js", () => ({ abortEmbeddedAgentRun: abortEmbeddedAgentRunMock, @@ -42,8 +45,8 @@ vi.mock("./abort-cutoff.js", () => ({ })); vi.mock("./abort.js", () => ({ - abortSessionRunTarget: abortSessionRunTargetMock, - formatAbortReplyText: vi.fn(() => "⚙️ Agent was aborted."), + abortSessionRunTargetWithOutcome: abortSessionRunTargetWithOutcomeMock, + formatAbortReplyText: formatAbortReplyTextMock, isAbortTrigger: vi.fn(() => false), setAbortMemory: vi.fn(), stopSubagentsForRequester: stopSubagentsForRequesterMock, @@ -136,6 +139,7 @@ describe("handleStopCommand target fallback", () => { beforeEach(() => { previousPluginRegistry = getActivePluginRegistry(); vi.clearAllMocks(); + abortSessionRunTargetWithOutcomeMock.mockReturnValue({ active: false, aborted: false }); persistAbortTargetEntryMock.mockResolvedValue(true); }); @@ -156,7 +160,7 @@ describe("handleStopCommand target fallback", () => { shouldContinue: false, reply: { text: "⚙️ Agent was aborted." }, }); - expect(abortSessionRunTargetMock).toHaveBeenCalledWith({ + expect(abortSessionRunTargetWithOutcomeMock).toHaveBeenCalledWith({ key: "agent:target:telegram:direct:123", sessionId: undefined, }); @@ -193,6 +197,23 @@ describe("handleStopCommand target fallback", () => { ); }); + it("reports a finalizing target without persisting abort state", async () => { + const params = buildStopParams(); + abortSessionRunTargetWithOutcomeMock.mockReturnValue({ active: true, aborted: false }); + formatAbortReplyTextMock.mockReturnValue( + "Agent reply is already finalizing and can no longer be aborted.", + ); + + const result = await handleStopCommand(params, true); + + expect(result).toEqual({ + shouldContinue: false, + reply: { text: "Agent reply is already finalizing and can no longer be aborted." }, + }); + expect(formatAbortReplyTextMock).toHaveBeenCalledWith(0, "finalizing"); + expect(persistAbortTargetEntryMock).not.toHaveBeenCalled(); + }); + it("rejects native stop commands from non-owner senders when the plugin enforces owner-only commands", async () => { registerOwnerEnforcingTelegramPlugin(); const params = buildStopParams(); @@ -230,7 +251,7 @@ describe("handleStopCommand target fallback", () => { shouldContinue: false, reply: { text: "You are not authorized to use this command." }, }); - expect(abortSessionRunTargetMock).not.toHaveBeenCalled(); + expect(abortSessionRunTargetWithOutcomeMock).not.toHaveBeenCalled(); expect(persistAbortTargetEntryMock).not.toHaveBeenCalled(); expect(createInternalHookEventMock).not.toHaveBeenCalled(); expect(stopSubagentsForRequesterMock).not.toHaveBeenCalled(); diff --git a/src/auto-reply/reply/dispatch-from-config.test.ts b/src/auto-reply/reply/dispatch-from-config.test.ts index 1898211cb174..7a723168317a 100644 --- a/src/auto-reply/reply/dispatch-from-config.test.ts +++ b/src/auto-reply/reply/dispatch-from-config.test.ts @@ -46,7 +46,12 @@ import { import { resolveRoutedDeliveryThreadId } from "./routed-delivery-thread.js"; import { buildTestCtx } from "./test-ctx.js"; -type AbortResult = { handled: boolean; aborted: boolean; stoppedSubagents?: number }; +type AbortResult = { + handled: boolean; + aborted: boolean; + rejectionReason?: "finalizing"; + stoppedSubagents?: number; +}; type ResolveInboundConversationParams = Parameters< NonNullable >[0]; @@ -4993,6 +4998,30 @@ describe("dispatchReplyFromConfig", () => { }); }); + it("reports when a fast abort is rejected during finalization", async () => { + mocks.tryFastAbortFromMessage.mockResolvedValue({ + handled: true, + aborted: false, + rejectionReason: "finalizing", + }); + const dispatcher = createDispatcher(); + const replyResolver = vi.fn(async () => ({ text: "hi" }) as ReplyPayload); + + await dispatchReplyFromConfig({ + ctx: buildTestCtx({ Provider: "telegram", Body: "/stop" }), + cfg: emptyConfig, + dispatcher, + replyResolver, + formatAbortReplyTextResolver: (_stopped, rejectionReason) => + rejectionReason === "finalizing" ? "already finalizing" : "aborted", + }); + + expect(replyResolver).not.toHaveBeenCalled(); + expect(dispatcher.sendFinalReply).toHaveBeenCalledWith({ + text: "already finalizing", + }); + }); + it("fast-abort reply includes stopped subagent count when provided", async () => { mocks.tryFastAbortFromMessage.mockResolvedValue({ handled: true, diff --git a/src/auto-reply/reply/dispatch-from-config.ts b/src/auto-reply/reply/dispatch-from-config.ts index adbfa2cd3145..f0bec05461b0 100644 --- a/src/auto-reply/reply/dispatch-from-config.ts +++ b/src/auto-reply/reply/dispatch-from-config.ts @@ -1542,7 +1542,9 @@ export async function dispatchReplyFromConfig( }; const getDispatchAbortSignal = () => { const operationSignal = dispatchReplyOperation?.abortSignal; - const upstreamSignal = params.replyOptions?.abortSignal; + // The operation mirrors upstream aborts until the backend commits its + // terminal outcome, then keeps delivery alive after freezeAbort(). + const upstreamSignal = operationSignal ? undefined : params.replyOptions?.abortSignal; if ( cachedDispatchAbortSignal && cachedDispatchAbortSignal.operationSignal === operationSignal && @@ -1550,7 +1552,7 @@ export async function dispatchReplyFromConfig( ) { return cachedDispatchAbortSignal.signal; } - const signal = composeAbortSignals(operationSignal, upstreamSignal); + const signal = operationSignal ?? upstreamSignal; cachedDispatchAbortSignal = { operationSignal, upstreamSignal, signal }; return signal; }; @@ -1588,6 +1590,7 @@ export async function dispatchReplyFromConfig( if (!dispatchReplyOperation) { return; } + dispatchReplyOperation.freezeAbort(); if (!dispatchReplyOperation.result) { dispatchReplyOperation.fail("run_failed", error); } @@ -2304,7 +2307,7 @@ export async function dispatchReplyFromConfig( let routedFinalCount = 0; if (!suppressDelivery) { const payload = { - text: formatAbortReplyTextResolver(fastAbort.stoppedSubagents), + text: formatAbortReplyTextResolver(fastAbort.stoppedSubagents, fastAbort.rejectionReason), } satisfies ReplyPayload; const result = await routeReplyToOriginating(payload); if (result) { diff --git a/src/auto-reply/reply/followup-runner.test.ts b/src/auto-reply/reply/followup-runner.test.ts index 9c61b2bd443d..9c688739a14b 100644 --- a/src/auto-reply/reply/followup-runner.test.ts +++ b/src/auto-reply/reply/followup-runner.test.ts @@ -2131,6 +2131,149 @@ describe("createFollowupRunner runtime config", () => { expect(call.abortSignal).toBe(fallbackCall.abortSignal); }); + it("suppresses a settled followup result after an accepted user abort", async () => { + let releaseFallback: () => void = () => undefined; + let releaseProgressRoute: () => void = () => undefined; + let markCandidateSettled: () => void = () => undefined; + const candidateSettled = new Promise((resolve) => { + markCandidateSettled = resolve; + }); + const fallbackRelease = new Promise((resolve) => { + releaseFallback = resolve; + }); + const progressRouteStarted = new Promise((resolve) => { + routeReplyMock.mockImplementationOnce( + async () => + await new Promise<{ ok: true }>((release) => { + releaseProgressRoute = () => release({ ok: true }); + resolve(); + }), + ); + }); + runEmbeddedAgentMock.mockImplementationOnce( + async (args: { onToolResult?: (payload: { text: string }) => Promise }) => { + void args.onToolResult?.({ text: "queued progress" }); + return { + payloads: [{ text: "late followup" }], + meta: {}, + }; + }, + ); + runWithModelFallbackMock.mockImplementationOnce( + async (params: { + run: ( + provider: string, + model: string, + options?: { isFinalFallbackAttempt?: boolean }, + ) => Promise<{ + payloads: Array<{ text: string }>; + meta: object; + }>; + }) => { + const result = await params.run("openai", "gpt-5.4", { + isFinalFallbackAttempt: false, + }); + markCandidateSettled(); + await fallbackRelease; + return { + result, + provider: "openai", + model: "gpt-5.4", + }; + }, + ); + const runner = createFollowupRunner({ + typing: createMockTypingController(), + typingMode: "instant", + sessionKey: "main", + defaultModel: "openai/gpt-5.4", + }); + + const pending = runner( + createQueuedRun({ + originatingChannel: "telegram", + originatingTo: "chat-1", + run: { + provider: "openai", + model: "gpt-5.4", + messageProvider: "telegram", + verboseLevel: "on", + }, + }), + ); + await candidateSettled; + await progressRouteStarted; + expect(requireLastMockCallArg(runEmbeddedAgentMock, "run embedded agent")).toMatchObject({ + isFinalFallbackAttempt: false, + }); + expect(replyRunRegistryForTest.abort("main")).toBe(true); + releaseFallback(); + let settled = false; + void pending.then(() => { + settled = true; + }); + await Promise.resolve(); + expect(settled).toBe(false); + releaseProgressRoute(); + await pending; + + expect(routeReplyMock).toHaveBeenCalledTimes(1); + expect(requireMockCallArg(routeReplyMock, 0).payload).toMatchObject({ + text: "queued progress", + }); + }); + + it("keeps a direct cancellation error from becoming a followup failure", async () => { + let rejectAttempt: (error: Error) => void = () => undefined; + let markAttemptStarted: () => void = () => undefined; + const attemptStarted = new Promise((resolve) => { + markAttemptStarted = resolve; + }); + runEmbeddedAgentMock.mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + rejectAttempt = reject; + markAttemptStarted(); + }), + ); + let operationResultDuringCompletion: + | import("./reply-run-registry.js").ReplyOperation["result"] + | undefined; + const runner = createFollowupRunner({ + typing: createMockTypingController(), + typingMode: "instant", + sessionKey: "main", + defaultModel: "openai/gpt-5.4", + }); + + const pending = runner( + createQueuedRun({ + originatingChannel: "telegram", + originatingTo: "chat-1", + queuedLifecycle: { + onComplete: () => { + operationResultDuringCompletion = replyRunRegistryForTest.get("main")?.result; + }, + }, + run: { + provider: "openai", + model: "gpt-5.4", + messageProvider: "telegram", + }, + }), + ); + await attemptStarted; + expect(replyRunRegistryForTest.abort("main")).toBe(true); + rejectAttempt(Object.assign(new Error("agent run aborted"), { name: "AbortError" })); + await pending; + + expect(routeReplyMock).not.toHaveBeenCalled(); + expect(operationResultDuringCompletion).toEqual({ + kind: "aborted", + code: "aborted_by_user", + }); + }); + it("keeps queued delivery correlations active during followup agent runs", async () => { const events: string[] = []; runEmbeddedAgentMock.mockImplementationOnce(async () => { diff --git a/src/auto-reply/reply/followup-runner.ts b/src/auto-reply/reply/followup-runner.ts index 00ce28f58116..eea9e6ac7fc2 100644 --- a/src/auto-reply/reply/followup-runner.ts +++ b/src/auto-reply/reply/followup-runner.ts @@ -1228,6 +1228,7 @@ export function createFollowupRunner(params: { timeoutMs: run.timeoutMs, runTimeoutOverrideMs: run.runTimeoutOverrideMs, runId, + isFinalFallbackAttempt: runOptions?.isFinalFallbackAttempt, abortSignal: runAbortSignal, deferTerminalLifecycle: true, onExecutionStarted: (info) => { @@ -1301,6 +1302,15 @@ export function createFollowupRunner(params: { settledLifecycleTerminal?.emit("end", runResult); throw runAbortSignal.reason; } + if ( + replyOperation.result?.kind === "aborted" && + replyOperation.result.code === "aborted_by_user" + ) { + settledLifecycleTerminal?.emit("end", runResult); + await drainProgressDeliveries(); + return; + } + replyOperation.freezeAbort(); const emitSettledLifecycleError = (error: Error, extraData?: Record) => { if (settledLifecycleTerminal) { settledLifecycleTerminal.emit("error", error, extraData); @@ -1353,7 +1363,20 @@ export function createFollowupRunner(params: { }); } } catch (err) { + if ( + replyOperation.result?.kind === "aborted" && + replyOperation.result.code === "aborted_by_user" + ) { + pendingLifecycleTerminal?.backstop.emit("error", err); + pendingLifecycleTerminal = undefined; + if (lifecycleGeneration !== getAgentEventLifecycleGeneration()) { + clearAgentRunContext(runId, lifecycleGeneration); + } + await drainProgressDeliveries(); + return; + } const message = formatErrorMessage(err); + replyOperation.freezeAbort(); replyOperation.fail("run_failed", err); pendingLifecycleTerminal?.backstop.emit("error", err); pendingLifecycleTerminal = undefined; diff --git a/src/auto-reply/reply/reply-run-registry.test.ts b/src/auto-reply/reply/reply-run-registry.test.ts index 300ca03eb1d2..b3391d9781e1 100644 --- a/src/auto-reply/reply/reply-run-registry.test.ts +++ b/src/auto-reply/reply/reply-run-registry.test.ts @@ -1,5 +1,6 @@ // Tests active reply run registry add, lookup, and cleanup behavior. import { afterEach, describe, expect, it, vi } from "vitest"; +import { createAgentRunRestartAbortError } from "../../agents/run-termination.js"; import { getDiagnosticSessionActivitySnapshot, resetDiagnosticRunActivityForTest, @@ -12,6 +13,7 @@ import { forceClearReplyRunBySessionId, isReplyRunActiveForSessionId, isReplyRunAbortableForCompaction, + isReplyRunAbortableForSignal, queueReplyRunMessage, REPLY_RUN_IDLE_SETTLE_TIMEOUT_MS, replyRunRegistry, @@ -382,6 +384,127 @@ describe("reply run registry", () => { expect(afterClear).toHaveBeenCalledTimes(1); }); + it("keeps retained terminal failures immutable across late aborts", () => { + const upstreamAbort = new AbortController(); + const cancel = vi.fn(); + const operation = createReplyOperation({ + sessionKey: "agent:main:failed-final", + sessionId: "session-failed-final", + resetTriggered: false, + upstreamAbortSignal: upstreamAbort.signal, + }); + operation.attachBackend({ + kind: "embedded", + cancel, + isStreaming: () => false, + isAbortable: () => true, + }); + operation.setPhase("running"); + operation.retainFailureUntilComplete(); + + operation.fail("run_failed", new Error("provider failed")); + upstreamAbort.abort(new Error("late upstream abort")); + + expect(operation.abortSignal.aborted).toBe(false); + expect(operation.abortByUser()).toBe(false); + expect(operation.abortForRestart()).toBe(false); + expect(operation.result).toMatchObject({ kind: "failed", code: "run_failed" }); + expect(operation.phase).toBe("failed"); + expect(cancel).not.toHaveBeenCalled(); + }); + + it("records upstream cancellation as an aborted operation", () => { + const upstreamAbort = new AbortController(); + const cancel = vi.fn(); + const operation = createReplyOperation({ + sessionKey: "agent:main:upstream-cancelled", + sessionId: "session-upstream-cancelled", + resetTriggered: false, + upstreamAbortSignal: upstreamAbort.signal, + }); + operation.attachBackend({ + kind: "embedded", + cancel, + isStreaming: () => true, + }); + operation.setPhase("running"); + + upstreamAbort.abort(new Error("caller cancelled")); + + expect(operation.result).toEqual({ kind: "aborted", code: "aborted_by_user" }); + expect(operation.phase).toBe("aborted"); + expect(operation.abortSignal.aborted).toBe(true); + expect(cancel).toHaveBeenCalledWith("user_abort"); + operation.complete(); + }); + + it("records upstream restart cancellation separately", () => { + const upstreamAbort = new AbortController(); + const cancel = vi.fn(); + const operation = createReplyOperation({ + sessionKey: "agent:main:upstream-restart", + sessionId: "session-upstream-restart", + resetTriggered: false, + upstreamAbortSignal: upstreamAbort.signal, + }); + operation.attachBackend({ + kind: "embedded", + cancel, + isStreaming: () => true, + }); + operation.setPhase("running"); + + upstreamAbort.abort(createAgentRunRestartAbortError()); + + expect(operation.result).toEqual({ kind: "aborted", code: "aborted_for_restart" }); + expect(operation.phase).toBe("aborted"); + expect(operation.abortSignal.aborted).toBe(true); + expect(cancel).toHaveBeenCalledWith("restart"); + operation.complete(); + }); + + it("clears queued ownership when the upstream signal is already aborted", () => { + const upstreamAbort = new AbortController(); + upstreamAbort.abort(new Error("caller already cancelled")); + + const operation = createReplyOperation({ + sessionKey: "agent:main:already-cancelled", + sessionId: "session-already-cancelled", + resetTriggered: false, + upstreamAbortSignal: upstreamAbort.signal, + }); + + expect(operation.result).toEqual({ kind: "aborted", code: "aborted_by_user" }); + expect(operation.phase).toBe("aborted"); + expect(operation.abortSignal.aborted).toBe(true); + expect(replyRunRegistry.isActive("agent:main:already-cancelled")).toBe(false); + }); + + it("does not cancel the backend twice when upstream abort follows a user abort", () => { + const upstreamAbort = new AbortController(); + const cancel = vi.fn(); + const operation = createReplyOperation({ + sessionKey: "agent:main:duplicate-cancel", + sessionId: "session-duplicate-cancel", + resetTriggered: false, + upstreamAbortSignal: upstreamAbort.signal, + }); + operation.attachBackend({ + kind: "embedded", + cancel, + isStreaming: () => true, + }); + operation.setPhase("running"); + + expect(operation.abortByUser()).toBe(true); + upstreamAbort.abort(createAgentRunRestartAbortError()); + + expect(operation.result).toEqual({ kind: "aborted", code: "aborted_by_user" }); + expect(cancel).toHaveBeenCalledTimes(1); + expect(cancel).toHaveBeenCalledWith("user_abort"); + operation.complete(); + }); + it("force-clears retained failed operations", () => { const operation = createReplyOperation({ sessionKey: "agent:main:main", @@ -428,6 +551,91 @@ describe("reply run registry", () => { } }); + it("rejects aborts while the attached backend is finalizing", () => { + let abortable = false; + const cancel = vi.fn(); + const operation = createReplyOperation({ + sessionKey: "agent:main:finalizing", + sessionId: "session-finalizing", + resetTriggered: false, + }); + operation.attachBackend({ + kind: "embedded", + cancel, + isStreaming: () => false, + isAbortable: () => abortable, + }); + operation.setPhase("running"); + + expect(replyRunRegistry.abort("agent:main:finalizing")).toBe(false); + expect(abortActiveReplyRuns({ mode: "all" })).toBe(false); + expect(operation.result).toBeNull(); + expect(cancel).not.toHaveBeenCalled(); + + abortable = true; + expect(replyRunRegistry.abort("agent:main:finalizing")).toBe(true); + expect(operation.result).toEqual({ kind: "aborted", code: "aborted_by_user" }); + expect(cancel).toHaveBeenCalledWith("user_abort"); + }); + + it("keeps finalizing reply bookkeeping through forced in-process restart", () => { + const cancel = vi.fn(); + const operation = createReplyOperation({ + sessionKey: "agent:main:restart-finalizing", + sessionId: "session-restart-finalizing", + resetTriggered: false, + }); + operation.attachBackend({ + kind: "embedded", + cancel, + isStreaming: () => false, + isAbortable: () => false, + }); + operation.setPhase("running"); + + expect(abortActiveReplyRuns({ mode: "all" })).toBe(false); + expect(replyRunRegistry.isActive("agent:main:restart-finalizing")).toBe(true); + expect(operation.result).toBeNull(); + expect(cancel).not.toHaveBeenCalled(); + + operation.complete(); + expect(replyRunRegistry.isActive("agent:main:restart-finalizing")).toBe(false); + }); + + it("keeps abort frozen after the backend detaches for reply delivery", () => { + const cancel = vi.fn(); + const upstreamAbort = new AbortController(); + const operation = createReplyOperation({ + sessionKey: "agent:main:delivery-finalizing", + sessionId: "session-delivery-finalizing", + resetTriggered: false, + upstreamAbortSignal: upstreamAbort.signal, + }); + const backend = { + kind: "embedded" as const, + cancel, + isStreaming: () => false, + isAbortable: () => false, + }; + operation.attachBackend(backend); + operation.setPhase("running"); + operation.freezeAbort(); + operation.detachBackend(backend); + + expect(isReplyRunAbortableForSignal(upstreamAbort.signal)).toBe(false); + expect(isReplyRunAbortableForSignal(new AbortController().signal)).toBe(true); + expect(replyRunRegistry.abort("agent:main:delivery-finalizing")).toBe(false); + expect(operation.result).toBeNull(); + expect(cancel).not.toHaveBeenCalled(); + + upstreamAbort.abort(); + expect(operation.abortSignal.aborted).toBe(false); + + operation.complete(); + expect(replyRunRegistry.isActive("agent:main:delivery-finalizing")).toBe(false); + expect(isReplyRunAbortableForSignal(upstreamAbort.signal)).toBe(false); + }); + it("clamps oversized wait timers instead of resolving idle waits immediately", async () => { vi.useFakeTimers(); try { diff --git a/src/auto-reply/reply/reply-run-registry.ts b/src/auto-reply/reply/reply-run-registry.ts index 1886014ea545..96816f67be19 100644 --- a/src/auto-reply/reply/reply-run-registry.ts +++ b/src/auto-reply/reply/reply-run-registry.ts @@ -1,6 +1,9 @@ // Tracks active reply runs so stop, queue, and status commands can coordinate. import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce"; -import { createAgentRunRestartAbortError } from "../../agents/run-termination.js"; +import { + createAgentRunRestartAbortError, + isAgentRunRestartAbortReason, +} from "../../agents/run-termination.js"; import { markDiagnosticEmbeddedRunEnded, markDiagnosticEmbeddedRunStarted, @@ -20,6 +23,7 @@ export type ReplyBackendHandle = { cancel(reason?: ReplyBackendCancelReason): void; isStreaming(): boolean; isStopped?: () => boolean; + isAbortable?: () => boolean; queueMessage?: (text: string) => Promise; /** * Compatibility-only hook so legacy "abort compacting runs" paths can still @@ -72,6 +76,8 @@ export type ReplyOperation = { updateSessionId(nextSessionId: string): void; attachBackend(handle: ReplyBackendHandle): void; detachBackend(handle: ReplyBackendHandle): void; + /** Reject later aborts after the backend has committed its terminal outcome. */ + freezeAbort(): void; /** * Keep a failed operation active until complete() releases the session lane. * Dispatch uses this while a user-visible failure payload still needs delivery. @@ -92,8 +98,8 @@ export type ReplyOperation = { timeout?: number | ReplyFollowupAdmissionBarrierTimeoutPolicy, ): void; fail(code: Exclude, cause?: unknown): void; - abortByUser(): void; - abortForRestart(): void; + abortByUser(): boolean; + abortForRestart(): boolean; }; export type ReplyRunRegistry = { @@ -227,6 +233,8 @@ function isReplyRunCompacting(operation: ReplyOperation): boolean { } const attachedBackendByOperation = new WeakMap(); +const abortFrozenOperations = new WeakSet(); +const operationsByUpstreamAbortSignal = new WeakMap(); const afterClearCallbacksByOperation = new WeakMap< ReplyOperation, Set<(sessionId: string) => void> @@ -236,6 +244,26 @@ function getAttachedBackend(operation: ReplyOperation): ReplyBackendHandle | und return attachedBackendByOperation.get(operation); } +function isReplyOperationAbortable(operation: ReplyOperation): boolean { + if (operation.result || abortFrozenOperations.has(operation)) { + return false; + } + const backend = getAttachedBackend(operation); + if (!backend?.isAbortable) { + return true; + } + try { + return backend.isAbortable(); + } catch { + return false; + } +} + +export function isReplyRunAbortableForSignal(signal: AbortSignal): boolean { + const operation = operationsByUpstreamAbortSignal.get(signal); + return operation ? isReplyOperationAbortable(operation) : true; +} + function isReplyBackendMessageInjectable(backend: ReplyBackendHandle): boolean { try { return backend.isStopped === undefined ? backend.isStreaming() : !backend.isStopped(); @@ -410,6 +438,15 @@ export function createReplyOperation(params: { let stateCleared = false; let retainFailureUntilComplete = false; let terminalRecovery = false; + const upstreamAbortSignal = params.upstreamAbortSignal; + let upstreamAbortHandler: (() => void) | undefined; + const detachUpstreamAbort = () => { + if (!upstreamAbortHandler) { + return; + } + upstreamAbortSignal?.removeEventListener("abort", upstreamAbortHandler); + upstreamAbortHandler = undefined; + }; const clearState = ( afterClearBarrier?: PromiseLike, @@ -419,6 +456,7 @@ export function createReplyOperation(params: { return; } stateCleared = true; + detachUpstreamAbort(); const registeredBarrier = afterClearBarrier ? registerFollowupAdmissionBarrier( sessionKey, @@ -455,26 +493,13 @@ export function createReplyOperation(params: { ) => { if (opts?.abortedCode && !result) { result = { kind: "aborted", code: opts.abortedCode }; + detachUpstreamAbort(); } phase = "aborted"; abortInternally(abortReason); getAttachedBackend(operation)?.cancel(reason); }; - if (params.upstreamAbortSignal) { - if (params.upstreamAbortSignal.aborted) { - abortInternally(params.upstreamAbortSignal.reason); - } else { - params.upstreamAbortSignal.addEventListener( - "abort", - () => { - abortInternally(params.upstreamAbortSignal?.reason); - }, - { once: true }, - ); - } - } - const operation: ReplyOperation = { get key() { return sessionKey; @@ -555,6 +580,10 @@ export function createReplyOperation(params: { attachedBackendByOperation.delete(operation); } }, + freezeAbort() { + abortFrozenOperations.add(operation); + detachUpstreamAbort(); + }, retainFailureUntilComplete() { retainFailureUntilComplete = true; }, @@ -577,6 +606,8 @@ export function createReplyOperation(params: { clearState(barrier, timeoutMs); }, fail(code, cause) { + abortFrozenOperations.add(operation); + detachUpstreamAbort(); if (!result) { result = { kind: "failed", code, cause }; phase = "failed"; @@ -586,6 +617,9 @@ export function createReplyOperation(params: { } }, abortByUser() { + if (!isReplyOperationAbortable(operation)) { + return false; + } const phaseBeforeAbort = phase; abortWithReason("user_abort", createUserAbortError(), { abortedCode: "aborted_by_user", @@ -593,8 +627,12 @@ export function createReplyOperation(params: { if (phaseBeforeAbort === "queued") { clearState(); } + return true; }, abortForRestart() { + if (!isReplyOperationAbortable(operation)) { + return false; + } const phaseBeforeAbort = phase; abortWithReason("restart", createAgentRunRestartAbortError(), { abortedCode: "aborted_for_restart", @@ -602,6 +640,7 @@ export function createReplyOperation(params: { if (phaseBeforeAbort === "queued") { clearState(); } + return true; }, }; @@ -610,6 +649,28 @@ export function createReplyOperation(params: { replyRunState.activeKeysBySessionId.set(currentSessionId, sessionKey); registerWaitSessionId(sessionKey, currentSessionId); markReplyRunDiagnosticWorkStarted({ sessionKey, sessionId: currentSessionId }); + if (upstreamAbortSignal) { + operationsByUpstreamAbortSignal.set(upstreamAbortSignal, operation); + const abortFromUpstream = () => { + if (result) { + return; + } + const restart = isAgentRunRestartAbortReason(upstreamAbortSignal.reason); + const phaseBeforeAbort = phase; + abortWithReason(restart ? "restart" : "user_abort", upstreamAbortSignal.reason, { + abortedCode: restart ? "aborted_for_restart" : "aborted_by_user", + }); + if (phaseBeforeAbort === "queued") { + clearState(); + } + }; + if (upstreamAbortSignal.aborted) { + abortFromUpstream(); + } else { + upstreamAbortHandler = abortFromUpstream; + upstreamAbortSignal.addEventListener("abort", upstreamAbortHandler, { once: true }); + } + } return operation; } @@ -644,8 +705,7 @@ export const replyRunRegistry: ReplyRunRegistry = { if (!operation) { return false; } - operation.abortByUser(); - return true; + return operation.abortByUser(); }, waitForIdle(sessionKey, timeoutMs, opts) { const normalizedSessionKey = normalizeOptionalString(sessionKey); @@ -718,6 +778,8 @@ export function isReplyRunActiveForSessionId(sessionId: string): boolean { export function isReplyRunAbortableForCompaction(sessionId: string): boolean { const operation = resolveReplyRunForCurrentSessionId(sessionId); + // Manual compaction uses this as a coordination gate: a finalizing run still + // needs to drain even when its frozen outcome rejects the abort itself. return Boolean(operation && operation.phase !== "queued"); } @@ -747,8 +809,7 @@ export function abortReplyRunBySessionId(sessionId: string): boolean { if (!operation) { return false; } - operation.abortByUser(); - return true; + return operation.abortByUser(); } export function forceClearReplyRunBySessionId(sessionId: string, cause?: unknown): boolean { @@ -826,14 +887,25 @@ export async function waitForReplyRunFollowupAdmission( } } -export function abortActiveReplyRuns(opts: { mode: "all" | "compacting" }): boolean { +export function abortActiveReplyRuns(opts: { + mode: "all" | "compacting"; + onAbortError?: (sessionId: string, error: unknown) => void; +}): boolean { let aborted = false; for (const operation of replyRunState.activeRunsByKey.values()) { if (opts.mode === "compacting" && !isReplyRunCompacting(operation)) { continue; } - operation.abortForRestart(); - aborted = true; + try { + if (operation.abortForRestart()) { + aborted = true; + } + } catch (error) { + if (operation.result?.kind === "aborted" && operation.result.code === "aborted_for_restart") { + aborted = true; + } + opts.onAbortError?.(operation.sessionId, error); + } } return aborted; } diff --git a/src/gateway/chat-abort.test.ts b/src/gateway/chat-abort.test.ts index 1118630a0eca..301c419ca2ad 100644 --- a/src/gateway/chat-abort.test.ts +++ b/src/gateway/chat-abort.test.ts @@ -239,12 +239,14 @@ describe("registerChatAbortController", () => { it("retains completed registrations until terminal persistence succeeds", async () => { const chatAbortControllers = new Map(); + const onRemoved = vi.fn(); const registration = registerChatAbortController({ chatAbortControllers, runId: "run-persisting", sessionId: "sess-1", sessionKey: "main", timeoutMs: 60_000, + onRemoved, }); let resolvePersistence: () => void = () => undefined; const persistence = new Promise((resolve) => { @@ -259,10 +261,12 @@ describe("registerChatAbortController", () => { registration.cleanup(); expect(chatAbortControllers.has("run-persisting")).toBe(true); + expect(onRemoved).not.toHaveBeenCalled(); resolvePersistence(); await persistence; await Promise.resolve(); expect(chatAbortControllers.has("run-persisting")).toBe(false); + expect(onRemoved).toHaveBeenCalledTimes(1); }); it("retains registrations when terminal lifecycle was observed before caller cleanup", () => { @@ -355,6 +359,27 @@ describe("abortChatRunById", () => { expect(payload.message).toBeUndefined(); }); + it("preserves finalizing runs when the owning reply operation rejects aborts", () => { + const { runId, sessionKey, entry, ops } = createAbortRunFixture({ + buffer: "completed reply", + entry: { + ...createActiveEntry("main"), + isAbortable: () => false, + }, + }); + + const result = abortChatRunById(ops, { runId, sessionKey, stopReason: "user" }); + + expect(result).toEqual({ aborted: false }); + expect(entry.controller.signal.aborted).toBe(false); + expect(ops.chatAbortControllers.get(runId)).toBe(entry); + expect(ops.chatRunBuffers.get(runId)).toBe("completed reply"); + expect(ops.chatAbortedRuns.has(runId)).toBe(false); + expect(ops.removeChatRun).not.toHaveBeenCalled(); + expect(ops.broadcast).not.toHaveBeenCalled(); + expect(ops.nodeSendToSession).not.toHaveBeenCalled(); + }); + it("aborts hidden internal runs without broadcasting chat events", () => { const sessionKey = "main"; const { runId, entry, ops } = createAbortRunFixture({ diff --git a/src/gateway/chat-abort.ts b/src/gateway/chat-abort.ts index 2fb540862f54..e7e8c07da3c0 100644 --- a/src/gateway/chat-abort.ts +++ b/src/gateway/chat-abort.ts @@ -50,6 +50,10 @@ export type ChatAbortControllerEntry = { projectSessionTerminalPersistence?: Promise; /** Caller completion requested cleanup before terminal lifecycle persistence settled. */ registrationCleanupRequested?: boolean; + /** False after the owning reply run commits a terminal outcome. */ + isAbortable?: (entry: ChatAbortControllerEntry) => boolean; + /** Runs once when this registration is actually removed. */ + onRemoved?: () => void; /** * Which RPC owns this registration. Absent (undefined) is treated as * `"chat-send"` so pre-existing callers that constructed entries without @@ -146,6 +150,8 @@ export function registerChatAbortController(params: { providerId?: string; authProviderId?: string; controlUiVisible?: boolean; + isAbortable?: (entry: ChatAbortControllerEntry) => boolean; + onRemoved?: () => void; kind?: ChatAbortControllerEntry["kind"]; lifecycleGeneration?: string; now?: number; @@ -156,7 +162,7 @@ export function registerChatAbortController(params: { const entry = params.chatAbortControllers.get(params.runId); if (entry?.controller === controller) { if (opts?.force === true) { - params.chatAbortControllers.delete(params.runId); + removeChatAbortControllerEntry(params.chatAbortControllers, params.runId, entry); return; } entry.registrationCleanupRequested = true; @@ -170,17 +176,17 @@ export function registerChatAbortController(params: { void persistence .then(() => { if (params.chatAbortControllers.get(params.runId)?.controller === controller) { - params.chatAbortControllers.delete(params.runId); + removeChatAbortControllerEntry(params.chatAbortControllers, params.runId, entry); } }) .catch(() => { if (params.chatAbortControllers.get(params.runId)?.controller === controller) { - params.chatAbortControllers.delete(params.runId); + removeChatAbortControllerEntry(params.chatAbortControllers, params.runId, entry); } }); return; } - params.chatAbortControllers.delete(params.runId); + removeChatAbortControllerEntry(params.chatAbortControllers, params.runId, entry); } }; @@ -209,6 +215,8 @@ export function registerChatAbortController(params: { providerId: normalizeProviderIdForActiveRun(params.providerId), authProviderId: normalizeProviderIdForActiveRun(params.authProviderId), controlUiVisible: params.controlUiVisible, + isAbortable: params.isAbortable, + onRemoved: params.onRemoved, projectSessionActive: true, kind: params.kind, }; @@ -451,6 +459,32 @@ function resolveDefaultGlobalAgentId(ops: ChatAbortOps): string | undefined { return cfg ? resolveDefaultAgentId(cfg) : undefined; } +export function isChatAbortControllerEntryAbortable(entry: ChatAbortControllerEntry): boolean { + try { + return entry.isAbortable?.(entry) !== false; + } catch { + return false; + } +} + +export function removeChatAbortControllerEntry( + entries: Map, + runId: string, + expectedEntry?: ChatAbortControllerEntry, +): boolean { + const entry = entries.get(runId); + if (!entry || (expectedEntry && entry !== expectedEntry)) { + return false; + } + entries.delete(runId); + try { + entry.onRemoved?.(); + } catch { + // Removal owns state cleanup even if a caller-provided release hook fails. + } + return true; +} + export function abortChatRunById( ops: ChatAbortOps, params: { @@ -467,6 +501,9 @@ export function abortChatRunById( if (active.sessionKey !== sessionKey) { return { aborted: false }; } + if (!isChatAbortControllerEntryAbortable(active)) { + return { aborted: false }; + } const bufferedText = ops.chatRunBuffers.get(runId); const partialText = bufferedText && bufferedText.trim() ? bufferedText : undefined; @@ -475,7 +512,7 @@ export function abortChatRunById( active.abortStopReason = stopReason; } active.controller.abort(createChatAbortSignalReason(stopReason)); - ops.chatAbortControllers.delete(runId); + removeChatAbortControllerEntry(ops.chatAbortControllers, runId, active); ops.clearChatRunState(runId); const removed = ops.removeChatRun(runId, runId, sessionKey); if (active.controlUiVisible !== false) { diff --git a/src/gateway/server-close.test.ts b/src/gateway/server-close.test.ts index c8a87b78289e..28c7e73c2e44 100644 --- a/src/gateway/server-close.test.ts +++ b/src/gateway/server-close.test.ts @@ -680,6 +680,39 @@ describe("createGatewayCloseHandler", () => { ).toBe(true); }); + it("does not abort a finalizing completed run when restart drain expires", async () => { + const controller = new AbortController(); + const chatAbortControllers = new Map([ + [ + "run-finalizing", + { + controller, + sessionId: "run-finalizing", + sessionKey: "session-finalizing", + projectSessionActive: false, + isAbortable: () => false, + startedAtMs: Date.now(), + expiresAtMs: Date.now() + 60_000, + }, + ], + ]); + const close = createGatewayCloseHandler( + createGatewayCloseTestDeps({ + chatAbortControllers, + }), + ); + + const result = await close({ + reason: "gateway restarting", + restartExpectedMs: 123, + drainTimeoutMs: 0, + }); + + expect(result.warnings).toContain("restart-reply-drain"); + expect(controller.signal.aborted).toBe(false); + expect(chatAbortControllers.has("run-finalizing")).toBe(true); + }); + it("marks active main sessions for restart recovery before aborting restart-drained runs", async () => { const events: string[] = []; const controller = new AbortController(); diff --git a/src/gateway/server-close.ts b/src/gateway/server-close.ts index 685138cd27f1..be29b4decff0 100644 --- a/src/gateway/server-close.ts +++ b/src/gateway/server-close.ts @@ -15,6 +15,8 @@ import type { PluginServicesHandle } from "../plugins/services.js"; import { abortTrackedChatRunById, type ChatAbortControllerEntry, + isChatAbortControllerEntryAbortable, + removeChatAbortControllerEntry, type RestartRecoveryCandidate, } from "./chat-abort.js"; import { @@ -406,10 +408,13 @@ async function markActiveRunsForRestartRecovery( function abortActiveRunsForRestart(params: RestartRunAbortParams): number { let aborted = 0; for (const [runId, entry] of listUnabortedRestartRuns(params.chatAbortControllers)) { + if (!isChatAbortControllerEntryAbortable(entry)) { + continue; + } if (entry.projectSessionActive === false) { entry.abortStopReason = "restart"; entry.controller.abort(createAgentRunRestartAbortError()); - params.chatAbortControllers.delete(runId); + removeChatAbortControllerEntry(params.chatAbortControllers, runId, entry); params.chatRunState.abortedRuns.set(runId, createChatAbortMarker()); params.chatRunState.clearRun(runId); const removed = params.removeChatRun(runId, runId, entry.sessionKey); diff --git a/src/gateway/server-maintenance.ts b/src/gateway/server-maintenance.ts index 989e56abcc30..a93a0563a594 100644 --- a/src/gateway/server-maintenance.ts +++ b/src/gateway/server-maintenance.ts @@ -7,6 +7,7 @@ import { cleanOldMedia } from "../media/store.js"; import { abortTrackedChatRunById, type ChatAbortControllerEntry, + removeChatAbortControllerEntry, type RestartRecoveryCandidate, } from "./chat-abort.js"; import { pruneStaleControlPlaneBuckets } from "./control-plane-rate-limit.js"; @@ -213,11 +214,11 @@ export function startGatewayMaintenanceTimers(params: { observedAt: entry.projectSessionTerminalObservedAt, }); } - params.chatAbortControllers.delete(runId); + removeChatAbortControllerEntry(params.chatAbortControllers, runId, entry); continue; } if (entry.projectSessionActive === false) { - params.chatAbortControllers.delete(runId); + removeChatAbortControllerEntry(params.chatAbortControllers, runId, entry); continue; } abortTrackedChatRunById(params, { diff --git a/src/gateway/server-methods/agent.ts b/src/gateway/server-methods/agent.ts index 8ed55229dc8a..3cd5b871267e 100644 --- a/src/gateway/server-methods/agent.ts +++ b/src/gateway/server-methods/agent.ts @@ -34,6 +34,11 @@ import { } from "../../agents/bash-tools.exec-approval-followup-state.js"; import { clearAllCliSessions } from "../../agents/cli-session.js"; import type { AgentCommandOpts } from "../../agents/command/types.js"; +import { + clearEmbeddedAgentRunAbortabilityForRunId, + isEmbeddedAgentRunAbortableForRunId, + retainEmbeddedAgentRunAbortabilityForRunId, +} from "../../agents/embedded-agent-runner/runs.js"; import { isTimeoutError } from "../../agents/failover-error.js"; import { resolveAgentAvatar, @@ -2630,6 +2635,8 @@ export const agentHandlers: GatewayRequestHandlers = { ownerDeviceId, providerId: activeModelProvider, authProviderId: activeAuthProvider, + isAbortable: () => isEmbeddedAgentRunAbortableForRunId(runId), + onRemoved: () => clearEmbeddedAgentRunAbortabilityForRunId(runId), controlUiVisible: !suppressVisibleSessionEffects, kind: "agent", lifecycleGeneration, @@ -2644,6 +2651,7 @@ export const agentHandlers: GatewayRequestHandlers = { return; } if (activeRunAbort.registered) { + retainEmbeddedAgentRunAbortabilityForRunId(runId); if (pendingChatRun) { context.addChatRun(runId, { ...pendingChatRun, @@ -2659,6 +2667,9 @@ export const agentHandlers: GatewayRequestHandlers = { ); } } + const cleanupActiveRunAbort = (opts?: { force?: boolean }) => { + activeRunAbort.cleanup(opts); + }; const resolvedThreadId = explicitThreadId ?? deliveryPlan.resolvedThreadId; // Confirmed only when the caller is the trusted in-process backend ACP @@ -2916,7 +2927,7 @@ export const agentHandlers: GatewayRequestHandlers = { runId, dedupeKeys: agentDedupeKeys, abortController: activeRunAbort.controller, - cleanupAbortController: activeRunAbort.cleanup, + cleanupAbortController: cleanupActiveRunAbort, respond, context, taskTrackingMode: dispatchTaskTrackingMode, @@ -2945,7 +2956,7 @@ export const agentHandlers: GatewayRequestHandlers = { }); } finally { if (!dispatched) { - activeRunAbort.cleanup({ force: true }); + cleanupActiveRunAbort({ force: true }); } } })(); diff --git a/src/gateway/server-methods/chat.abort-persistence.test.ts b/src/gateway/server-methods/chat.abort-persistence.test.ts index 55fc868ea7c8..4c68aedc9092 100644 --- a/src/gateway/server-methods/chat.abort-persistence.test.ts +++ b/src/gateway/server-methods/chat.abort-persistence.test.ts @@ -401,6 +401,44 @@ describe("chat abort transcript persistence", () => { expect(runBPersisted).toBeUndefined(); }); + it("does not persist partials from finalizing runs that reject a session abort", async () => { + const { transcriptPath, sessionId } = await createTranscriptFixture( + "openclaw-chat-abort-finalizing-", + ); + const respond = vi.fn(); + const finalizingRun = { + ...createActiveRun("main", { sessionId }), + isAbortable: () => false, + }; + const context = createChatAbortContext({ + chatAbortControllers: new Map([ + ["run-aborted", createActiveRun("main", { sessionId })], + ["run-finalizing", finalizingRun], + ]), + chatRunBuffers: new Map([ + ["run-aborted", "Aborted partial"], + ["run-finalizing", "Completed reply"], + ]), + }); + + await invokeChatAbortHandler({ + handler: chatHandlers["chat.abort"], + context, + request: { sessionKey: "main" }, + respond, + }); + + const [ok, payload] = requireLastRespondCall(respond); + expect(ok).toBe(true); + expectAbortPayload(payload, { runIds: ["run-aborted"] }); + expect(finalizingRun.controller.signal.aborted).toBe(false); + expect(context.chatAbortControllers.get("run-finalizing")).toBe(finalizingRun); + + const lines = await readTranscriptLines(transcriptPath); + expect(findMessageWithIdempotencyKey(lines, "run-aborted:assistant")).toBeDefined(); + expect(findMessageWithIdempotencyKey(lines, "run-finalizing:assistant")).toBeUndefined(); + }); + it("persists /stop partials with stop-command metadata", async () => { const { transcriptPath, sessionId } = await createTranscriptFixture("openclaw-chat-stop-"); const respond = vi.fn(); diff --git a/src/gateway/server-methods/chat.ts b/src/gateway/server-methods/chat.ts index cbfa414940f5..da8c28ab83b0 100644 --- a/src/gateway/server-methods/chat.ts +++ b/src/gateway/server-methods/chat.ts @@ -55,6 +55,7 @@ import { type ReplyPayload, } from "../../auto-reply/reply-payload.js"; import { createReplyDispatcher } from "../../auto-reply/reply/reply-dispatcher.js"; +import { isReplyRunAbortableForSignal } from "../../auto-reply/reply/reply-run-registry.js"; import { stageSandboxMedia, type StageSandboxMediaResult, @@ -2493,10 +2494,11 @@ async function abortChatRunsForSessionKeyWithPartials(params: { } const res = { aborted: runIds.length > 0, runIds, unauthorized: false }; if (res.aborted) { + const abortedRunIds = new Set(runIds); await persistAbortedPartials({ context: params.context, sessionKey: params.persistSessionKey ?? params.sessionKey, - snapshots, + snapshots: snapshots.filter((snapshot) => abortedRunIds.has(snapshot.runId)), }); } return res; @@ -3692,6 +3694,7 @@ export const chatHandlers: GatewayRequestHandlers = { ownerDeviceId: normalizeOptionalText(client?.connect?.device?.id), providerId: resolvedSessionModel.provider, authProviderId: resolvedSessionAuthProvider, + isAbortable: (active) => isReplyRunAbortableForSignal(active.controller.signal), kind: "chat-send", lifecycleGeneration, }); diff --git a/src/gateway/server-runtime-subscriptions.ts b/src/gateway/server-runtime-subscriptions.ts index 282276892043..8ede93af2f90 100644 --- a/src/gateway/server-runtime-subscriptions.ts +++ b/src/gateway/server-runtime-subscriptions.ts @@ -4,7 +4,11 @@ import { onHeartbeatEvent } from "../infra/heartbeat-events.js"; import { onSessionLifecycleEvent } from "../sessions/session-lifecycle-events.js"; import { onInternalSessionTranscriptUpdate } from "../sessions/transcript-events.js"; import { createLazyPromise } from "../shared/lazy-runtime.js"; -import type { ChatAbortControllerEntry, RestartRecoveryCandidate } from "./chat-abort.js"; +import { + type ChatAbortControllerEntry, + removeChatAbortControllerEntry, + type RestartRecoveryCandidate, +} from "./chat-abort.js"; import type { ChatRunState, SessionEventSubscriberRegistry, @@ -63,7 +67,11 @@ export function startGatewayEventSubscriptions(params: { entry.registrationCleanupRequested === true && !entry.projectSessionTerminalPersistence ) { - params.chatAbortControllers.delete(candidateRunId); + removeChatAbortControllerEntry( + params.chatAbortControllers, + candidateRunId, + entry, + ); } }); } @@ -99,7 +107,11 @@ export function startGatewayEventSubscriptions(params: { .catch(() => undefined) .then(() => { if (params.chatAbortControllers.get(candidateRunId) === entry) { - params.chatAbortControllers.delete(candidateRunId); + removeChatAbortControllerEntry( + params.chatAbortControllers, + candidateRunId, + entry, + ); } }); }