diff --git a/src/browser/features/ChatInput/index.tsx b/src/browser/features/ChatInput/index.tsx index 84e44a92ebc..deaa0887eab 100644 --- a/src/browser/features/ChatInput/index.tsx +++ b/src/browser/features/ChatInput/index.tsx @@ -78,6 +78,7 @@ import { } from "@/browser/utils/workflowRunMessages"; import { Button } from "@/browser/components/Button/Button"; import { CUSTOM_EVENTS } from "@/common/constants/events"; +import { useChatErrorToasts } from "@/browser/utils/chatErrorToasts"; import { EXPERIMENT_IDS } from "@/common/constants/experiments"; import { extractInlineSkillReferenceCandidates } from "@/browser/utils/agentSkills/inlineSkillReferences"; import { @@ -1467,23 +1468,7 @@ const ChatInputInner: React.FC = (props) => { window.removeEventListener(CUSTOM_EVENTS.THINKING_LEVEL_TOAST, handler as EventListener); }, [variant, props, pushToast]); - // Show the backend's one-shot child-budget warning on the matching parent workspace. - useEffect(() => { - if (variant !== "workspace") return; - - const handler = (event: Event) => { - const detail = (event as CustomEvent<{ workspaceId: string; message: string }>).detail; - if (detail?.workspaceId !== workspaceId || !detail.message) { - return; - } - - pushToast({ type: "error", message: detail.message }); - }; - - window.addEventListener(CUSTOM_EVENTS.GOAL_CHILD_BUDGET_TOAST, handler as EventListener); - return () => - window.removeEventListener(CUSTOM_EVENTS.GOAL_CHILD_BUDGET_TOAST, handler as EventListener); - }, [variant, workspaceId, pushToast]); + useChatErrorToasts(workspaceId, toast?.message ?? null, pushToast); // Show toast feedback for analytics rebuild command palette action. useEffect(() => { diff --git a/src/browser/features/Messages/ChatBarrier/RetryBarrier.test.tsx b/src/browser/features/Messages/ChatBarrier/RetryBarrier.test.tsx index 4a87d6cb9cb..60242e66661 100644 --- a/src/browser/features/Messages/ChatBarrier/RetryBarrier.test.tsx +++ b/src/browser/features/Messages/ChatBarrier/RetryBarrier.test.tsx @@ -72,6 +72,7 @@ function createDeferred() { let resumeStreamResult: ResumeStreamResult = { success: true, data: { started: true } }; let previousAutoRetryEnabled = false; const resumeStream = mock((_input: unknown) => Promise.resolve(resumeStreamResult)); +const interruptStream = mock((_input: unknown) => Promise.resolve({ success: true as const })); const setAutoRetryEnabled = mock((input: unknown) => { if ( typeof input === "object" && @@ -107,6 +108,7 @@ void mock.module("@/browser/contexts/API", () => ({ api: { workspace: { resumeStream, + interruptStream, setAutoRetryEnabled, }, }, @@ -152,6 +154,7 @@ describe("RetryBarrier", () => { resumeStreamResult = { success: true, data: { started: true } }; previousAutoRetryEnabled = false; resumeStream.mockClear(); + interruptStream.mockClear(); setAutoRetryEnabled.mockClear(); }); @@ -394,4 +397,27 @@ describe("RetryBarrier", () => { }); expect(resumeStream).toHaveBeenCalledTimes(1); }); + + test("the Stop button opts out of auto-retry inside the same attention-retiring Stop as its shortcut", async () => { + currentWorkspaceState = createWorkspaceState({ + autoRetryStatus: { + type: "auto-retry-scheduled", + attempt: 1, + delayMs: 5_000, + scheduledAt: Date.now(), + }, + }); + + const view = render(); + + fireEvent.click(view.getByRole("button", { name: /^Stop/ })); + + await waitFor(() => expect(interruptStream).toHaveBeenCalledTimes(1)); + expect(interruptStream).toHaveBeenCalledWith({ + workspaceId: "ws-1", + options: { disableAutoRetry: true, retireBashMonitorAttention: true }, + }); + // No separate opt-out call: it would release the retry idle gate ahead of the Stop. + expect(setAutoRetryEnabled).not.toHaveBeenCalled(); + }); }); diff --git a/src/browser/features/Messages/ChatBarrier/RetryBarrier.tsx b/src/browser/features/Messages/ChatBarrier/RetryBarrier.tsx index e3c954092a0..514736647d2 100644 --- a/src/browser/features/Messages/ChatBarrier/RetryBarrier.tsx +++ b/src/browser/features/Messages/ChatBarrier/RetryBarrier.tsx @@ -8,6 +8,7 @@ import { KEYBINDS, formatKeybind } from "@/browser/utils/ui/keybinds"; import { VIM_ENABLED_KEY } from "@/common/constants/storage"; import { getSendOptionsFromStorage } from "@/browser/utils/messages/sendOptions"; import { applyCompactionOverrides } from "@/browser/utils/messages/compactionOptions"; +import { stopStream } from "@/browser/utils/stopStream"; import { formatSendMessageError } from "@/common/utils/errors/formatSendError"; import { getErrorMessage } from "@/common/utils/errors"; @@ -233,10 +234,11 @@ export const RetryBarrier: React.FC = (props) => { } }; - const handleStopAutoRetry = () => { + const handleStopAutoRetry = async () => { setCountdown(0); setManualRetryError(null); - void api?.workspace.setAutoRetryEnabled?.({ workspaceId: props.workspaceId, enabled: false }); + if (!api) return; + await stopStream(api, props.workspaceId, { disableAutoRetry: true }); }; const lastMessage = getLastMainRetryCandidateMessage(workspaceState.messages); @@ -301,7 +303,9 @@ export const RetryBarrier: React.FC = (props) => { actionButton = ( diff --git a/src/browser/features/Messages/ChatBarrier/StreamingBarrier.test.tsx b/src/browser/features/Messages/ChatBarrier/StreamingBarrier.test.tsx index 4978ca1dbc8..fa6cd89c640 100644 --- a/src/browser/features/Messages/ChatBarrier/StreamingBarrier.test.tsx +++ b/src/browser/features/Messages/ChatBarrier/StreamingBarrier.test.tsx @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; -import { cleanup, fireEvent, render } from "@testing-library/react"; +import { cleanup, fireEvent, render, waitFor } from "@testing-library/react"; import { GlobalWindow } from "happy-dom"; import type * as WorkspaceStoreModule from "@/browser/stores/WorkspaceStore"; @@ -163,7 +163,7 @@ describe("StreamingBarrier", () => { globalThis.document = undefined as unknown as Document; }); - test("clicking stop during normal streaming interrupts with default options", () => { + test("clicking stop during normal streaming interrupts with default options", async () => { currentWorkspaceState = createWorkspaceState({ canInterrupt: true, isCompacting: false, @@ -175,12 +175,17 @@ describe("StreamingBarrier", () => { fireEvent.click(view.getByRole("button", { name: "Stop streaming" })); - expect(setAutoRetryEnabled).toHaveBeenCalledWith({ workspaceId: "ws-1", enabled: false }); expect(setInterrupting).toHaveBeenCalledWith("ws-1"); - expect(interruptStream).toHaveBeenCalledWith({ workspaceId: "ws-1" }); + await waitFor(() => + expect(interruptStream).toHaveBeenCalledWith({ + workspaceId: "ws-1", + options: { disableAutoRetry: true, retireBashMonitorAttention: true }, + }) + ); + expect(setAutoRetryEnabled).not.toHaveBeenCalled(); }); - test("clicking stop during stream-start interrupts without setting interrupting state", () => { + test("clicking stop during stream-start interrupts without setting interrupting state", async () => { currentWorkspaceState = createWorkspaceState({ canInterrupt: false, pendingStreamStartTime: Date.now(), @@ -195,9 +200,13 @@ describe("StreamingBarrier", () => { fireEvent.click(stopButton); - expect(setAutoRetryEnabled).toHaveBeenCalledWith({ workspaceId: "ws-1", enabled: false }); expect(setInterrupting).not.toHaveBeenCalled(); - expect(interruptStream).toHaveBeenCalledWith({ workspaceId: "ws-1" }); + await waitFor(() => + expect(interruptStream).toHaveBeenCalledWith({ + workspaceId: "ws-1", + options: { disableAutoRetry: true, retireBashMonitorAttention: true }, + }) + ); }); test("shows the barrier immediately on first appearance", () => { @@ -358,13 +367,14 @@ describe("StreamingBarrier", () => { fireEvent.click(view.getByRole("button", { name: "Stop streaming" })); - expect(setAutoRetryEnabled).toHaveBeenCalledWith({ workspaceId: "ws-1", enabled: false }); + // The compaction-cancel flow owns the retry opt-out along with its Stop. expect(onCancelCompaction).toHaveBeenCalledTimes(1); + expect(setAutoRetryEnabled).not.toHaveBeenCalled(); expect(setInterrupting).not.toHaveBeenCalled(); expect(interruptStream).not.toHaveBeenCalled(); }); - test("clicking stop during compaction falls back to abandonPartial interrupt", () => { + test("clicking stop during compaction falls back to abandonPartial interrupt", async () => { currentWorkspaceState = createWorkspaceState({ canInterrupt: true, isCompacting: true, @@ -374,12 +384,13 @@ describe("StreamingBarrier", () => { fireEvent.click(view.getByRole("button", { name: "Stop streaming" })); - expect(setAutoRetryEnabled).toHaveBeenCalledWith({ workspaceId: "ws-1", enabled: false }); expect(setInterrupting).not.toHaveBeenCalled(); - expect(interruptStream).toHaveBeenCalledWith({ - workspaceId: "ws-1", - options: { abandonPartial: true }, - }); + await waitFor(() => + expect(interruptStream).toHaveBeenCalledWith({ + workspaceId: "ws-1", + options: { abandonPartial: true, disableAutoRetry: true, retireBashMonitorAttention: true }, + }) + ); }); test("resets to new workspace text immediately on workspace switch", () => { diff --git a/src/browser/features/Messages/ChatBarrier/StreamingBarrier.tsx b/src/browser/features/Messages/ChatBarrier/StreamingBarrier.tsx index 6e97be26da9..24f4c9316f7 100644 --- a/src/browser/features/Messages/ChatBarrier/StreamingBarrier.tsx +++ b/src/browser/features/Messages/ChatBarrier/StreamingBarrier.tsx @@ -14,6 +14,7 @@ import { import { getDefaultModel } from "@/browser/hooks/useModelsFromSettings"; import { useSettings } from "@/browser/contexts/SettingsContext"; import { useAPI } from "@/browser/contexts/API"; +import { stopStream } from "@/browser/utils/stopStream"; type StreamingPhase = | "starting" // Message sent, waiting for stream-start @@ -264,8 +265,6 @@ export const StreamingBarrier: React.FC = ({ return; } - void api.workspace.setAutoRetryEnabled?.({ workspaceId, enabled: false }); - if (phase === "compacting") { // Reuse the established compaction-cancel flow from keyboard shortcuts so we keep // edit restoration + follow-up content behavior consistent across input methods. @@ -274,10 +273,7 @@ export const StreamingBarrier: React.FC = ({ return; } - void api.workspace.interruptStream({ - workspaceId, - options: { abandonPartial: true }, - }); + void stopStream(api, workspaceId, { abandonPartial: true, disableAutoRetry: true }); return; } @@ -285,7 +281,7 @@ export const StreamingBarrier: React.FC = ({ storeRaw.setInterrupting(workspaceId); } - void api.workspace.interruptStream({ workspaceId }); + void stopStream(api, workspaceId, { disableAutoRetry: true }); }; // Show settings hint during compaction if no custom compaction model is configured diff --git a/src/browser/hooks/useAIViewKeybinds.test.tsx b/src/browser/hooks/useAIViewKeybinds.test.tsx index 8e47a1534ab..9eff8111663 100644 --- a/src/browser/hooks/useAIViewKeybinds.test.tsx +++ b/src/browser/hooks/useAIViewKeybinds.test.tsx @@ -1,6 +1,6 @@ import type { ReactNode, RefObject } from "react"; import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; -import { cleanup, renderHook } from "@testing-library/react"; +import { cleanup, renderHook, waitFor } from "@testing-library/react"; import { copyFile, readFile, rm, writeFile } from "node:fs/promises"; import { randomUUID } from "node:crypto"; import { dirname, join } from "node:path"; @@ -90,7 +90,7 @@ describe("useAIViewKeybinds", () => { isolatedModulePaths = []; }); - test("Escape interrupts an active stream in normal mode", () => { + test("Escape interrupts an active stream in normal mode", async () => { const interruptStream = mock(() => Promise.resolve({ success: true as const, data: undefined }) ); @@ -124,7 +124,7 @@ describe("useAIViewKeybinds", () => { }) ); - expect(interruptStream.mock.calls.length).toBe(1); + await waitFor(() => expect(interruptStream.mock.calls.length).toBe(1)); }); test("Escape does not interrupt when the event target is an ", () => { @@ -168,7 +168,7 @@ describe("useAIViewKeybinds", () => { expect(interruptStream.mock.calls.length).toBe(0); }); - test("Escape interrupts when an editable element opts in", () => { + test("Escape interrupts when an editable element opts in", async () => { const interruptStream = mock(() => Promise.resolve({ success: true as const, data: undefined }) ); @@ -207,10 +207,10 @@ describe("useAIViewKeybinds", () => { }) ); - expect(interruptStream.mock.calls.length).toBe(1); + await waitFor(() => expect(interruptStream.mock.calls.length).toBe(1)); }); - test("Ctrl+C interrupts in vim mode even when an is focused", () => { + test("Ctrl+C interrupts in vim mode even when an is focused", async () => { const interruptStream = mock(() => Promise.resolve({ success: true as const, data: undefined }) ); @@ -249,7 +249,56 @@ describe("useAIViewKeybinds", () => { }) ); - expect(interruptStream.mock.calls.length).toBe(1); + await waitFor(() => expect(interruptStream.mock.calls.length).toBe(1)); + }); + + test("Escape on the retry barrier opts out of auto-retry inside the Stop itself", async () => { + const interruptStream = mock(() => + Promise.resolve({ success: true as const, data: undefined }) + ); + const setAutoRetryEnabled = mock(() => + Promise.resolve({ + success: true as const, + data: { previousEnabled: true, enabled: false }, + }) + ); + currentClientMock = { + workspace: { + interruptStream, + setAutoRetryEnabled, + }, + }; + + const chatInputAPI: RefObject = { current: null }; + + renderUseAIViewKeybinds({ + workspaceId: "ws", + canInterrupt: false, + showRetryBarrier: true, + chatInputAPI, + jumpToBottom: () => undefined, + loadOlderHistory: null, + handleOpenTerminal: () => undefined, + handleOpenInEditor: () => undefined, + aggregator: undefined, + setEditingMessage: () => undefined, + vimEnabled: false, + }); + + document.body.dispatchEvent( + new window.KeyboardEvent("keydown", { + key: "Escape", + bubbles: true, + cancelable: true, + }) + ); + + await waitFor(() => expect(interruptStream.mock.calls.length).toBe(1)); + expect(interruptStream).toHaveBeenCalledWith({ + workspaceId: "ws", + options: { disableAutoRetry: true, retireBashMonitorAttention: true }, + }); + expect(setAutoRetryEnabled).not.toHaveBeenCalled(); }); test.each([ diff --git a/src/browser/hooks/useAIViewKeybinds.ts b/src/browser/hooks/useAIViewKeybinds.ts index bb312b2a573..a328bbf45cc 100644 --- a/src/browser/hooks/useAIViewKeybinds.ts +++ b/src/browser/hooks/useAIViewKeybinds.ts @@ -12,6 +12,7 @@ import { } from "@/browser/utils/ui/keybinds"; import type { StreamingMessageAggregator } from "@/browser/utils/messages/StreamingMessageAggregator"; import { isCompactingStream, cancelCompaction } from "@/browser/utils/compaction/handler"; +import { stopStream } from "@/browser/utils/stopStream"; import { useAPI } from "@/browser/contexts/API"; import type { EditingMessageState } from "@/browser/utils/chatEditing"; @@ -111,7 +112,6 @@ export function useAIViewKeybinds({ if (api) { void cancelCompaction(api, workspaceId, aggregator, setEditingMessage); } - void api?.workspace.setAutoRetryEnabled?.({ workspaceId, enabled: false }); return; } @@ -120,8 +120,9 @@ export function useAIViewKeybinds({ // Non-vim mode: Esc interrupts (except when typing in inputs, unless explicitly opted in) if (canInterrupt || showRetryBarrier) { e.preventDefault(); - void api?.workspace.setAutoRetryEnabled?.({ workspaceId, enabled: false }); - void api?.workspace.interruptStream({ workspaceId }); + if (api) { + void stopStream(api, workspaceId, { disableAutoRetry: true }); + } return; } } diff --git a/src/browser/utils/chatErrorToasts.test.tsx b/src/browser/utils/chatErrorToasts.test.tsx new file mode 100644 index 00000000000..3776f70aa2f --- /dev/null +++ b/src/browser/utils/chatErrorToasts.test.tsx @@ -0,0 +1,182 @@ +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { act, cleanup, renderHook } from "@testing-library/react"; +import { GlobalWindow } from "happy-dom"; +import { StrictMode } from "react"; +import { + dismissChatError, + peekChatError, + publishChatError, + useChatErrorToasts, +} from "./chatErrorToasts"; + +describe("useChatErrorToasts", () => { + beforeEach(() => { + const domWindow = new GlobalWindow() as unknown as Window & typeof globalThis; + globalThis.window = domWindow; + globalThis.document = domWindow.document; + }); + + afterEach(() => { + cleanup(); + globalThis.window = undefined as unknown as Window & typeof globalThis; + globalThis.document = undefined as unknown as Document; + }); + + interface Props { + workspaceId: string | null; + visible: string | null; + } + + /** A chat input whose single toast slot the test drives: `show` renders a toast, `dismiss` clears it. */ + function mountInput(workspaceId: string | null, options?: { strict?: boolean }) { + const shown: string[] = []; + const pushToast = (toast: { message: string }) => { + shown.push(toast.message); + }; + const initialProps: Props = { workspaceId, visible: null }; + const rendered = renderHook( + (props: Props) => useChatErrorToasts(props.workspaceId, props.visible, pushToast), + { initialProps, wrapper: options?.strict ? StrictMode : undefined } + ); + return { + shown, + ...rendered, + show: (message: string) => rendered.rerender({ workspaceId, visible: message }), + dismiss: () => rendered.rerender({ workspaceId, visible: null }), + }; + } + + function drain(workspaceId: string) { + for (let next = peekChatError(workspaceId); next != null; next = peekChatError(workspaceId)) { + dismissChatError(workspaceId, next); + } + } + + test("an error published while no input for the workspace is mounted is shown once it mounts", () => { + publishChatError("ws-a", "Stop could not be recorded"); + + const input = mountInput("ws-a"); + + expect(input.shown).toEqual(["Stop could not be recorded"]); + // Queued until the toast has been rendered and dismissed, not merely pushed. + expect(peekChatError("ws-a")).toBe("Stop could not be recorded"); + input.show("Stop could not be recorded"); + input.dismiss(); + expect(peekChatError("ws-a")).toBeUndefined(); + }); + + test("an error published while the input is mounted is shown immediately", () => { + const input = mountInput("ws-b"); + + act(() => { + publishChatError("ws-b", "Child exceeded the goal budget"); + }); + + expect(input.shown).toEqual(["Child exceeded the goal budget"]); + drain("ws-b"); + }); + + test("errors for another workspace stay retained for that workspace's input", () => { + const input = mountInput("ws-c"); + + act(() => { + publishChatError("ws-d", "for d"); + }); + + expect(input.shown).toEqual([]); + const other = mountInput("ws-d"); + expect(other.shown).toEqual(["for d"]); + drain("ws-d"); + }); + + test("an error published after the input unmounts waits for the next mount", () => { + const first = mountInput("ws-e"); + first.unmount(); + + publishChatError("ws-e", "late Stop failure"); + expect(first.shown).toEqual([]); + + const second = mountInput("ws-e"); + expect(second.shown).toEqual(["late Stop failure"]); + drain("ws-e"); + }); + + test("errors retained together are shown one toast at a time, the next after a dismissal", () => { + publishChatError("ws-f", "first"); + publishChatError("ws-f", "second"); + + const input = mountInput("ws-f"); + expect(input.shown).toEqual(["first"]); + + input.show("first"); + expect(input.shown).toEqual(["first"]); + input.dismiss(); + expect(input.shown).toEqual(["first", "second"]); + + input.show("second"); + input.dismiss(); + expect(input.shown).toEqual(["first", "second"]); + expect(peekChatError("ws-f")).toBeUndefined(); + }); + + test("an error published while another toast is visible waits for that toast to be dismissed", () => { + const input = mountInput("ws-g"); + input.show("Not connected to server"); + + act(() => { + publishChatError("ws-g", "Stop could not be recorded"); + }); + expect(input.shown).toEqual([]); + + input.dismiss(); + expect(input.shown).toEqual(["Stop could not be recorded"]); + input.show("Stop could not be recorded"); + input.dismiss(); + expect(peekChatError("ws-g")).toBeUndefined(); + }); + + test("a pushed error another toast rendered over is pushed again once that toast is dismissed", () => { + const input = mountInput("ws-h"); + act(() => { + publishChatError("ws-h", "lost in a batch"); + }); + expect(input.shown).toEqual(["lost in a batch"]); + + // The input rendered a different toast (a same-tick setToast won the batch), never ours. + input.show("something else"); + input.dismiss(); + + expect(input.shown).toEqual(["lost in a batch", "lost in a batch"]); + input.show("lost in a batch"); + input.dismiss(); + expect(peekChatError("ws-h")).toBeUndefined(); + }); + + test("a displayed error that another toast replaced is pushed again once that toast is dismissed", () => { + const input = mountInput("ws-j"); + act(() => { + publishChatError("ws-j", "Stop could not be recorded"); + }); + input.show("Stop could not be recorded"); + // A later toast took the slot before the user dismissed ours. + input.show("Thinking level: high"); + input.dismiss(); + + expect(input.shown).toEqual(["Stop could not be recorded", "Stop could not be recorded"]); + input.show("Stop could not be recorded"); + input.dismiss(); + expect(peekChatError("ws-j")).toBeUndefined(); + }); + + test("StrictMode's replayed effect re-pushes the same error instead of consuming the next one", () => { + publishChatError("ws-i", "first"); + publishChatError("ws-i", "second"); + + const input = mountInput("ws-i", { strict: true }); + + expect(input.shown.length).toBeGreaterThan(0); + expect(new Set(input.shown)).toEqual(new Set(["first"])); + expect(peekChatError("ws-i")).toBe("first"); + drain("ws-i"); + }); +}); diff --git a/src/browser/utils/chatErrorToasts.ts b/src/browser/utils/chatErrorToasts.ts new file mode 100644 index 00000000000..811e6f98291 --- /dev/null +++ b/src/browser/utils/chatErrorToasts.ts @@ -0,0 +1,75 @@ +import { useEffect, useRef } from "react"; + +/** + * Error toasts addressed to a workspace's chat input (a child exhausting the parent's goal budget, + * a Stop the backend could not record). Retained until that input has shown and dismissed them: the + * error can land after the user switched workspaces (a Stop settles asynchronously), when no input + * for that workspace is mounted, and the input renders a single toast at a time. + */ +const pendingByWorkspace = new Map(); +const listenersByWorkspace = new Map void>>(); + +export function publishChatError(workspaceId: string, message: string): void { + const pending = pendingByWorkspace.get(workspaceId) ?? []; + pending.push(message); + pendingByWorkspace.set(workspaceId, pending); + for (const listener of listenersByWorkspace.get(workspaceId) ?? []) { + listener(); + } +} + +export function peekChatError(workspaceId: string): string | undefined { + return pendingByWorkspace.get(workspaceId)?.[0]; +} + +export function dismissChatError(workspaceId: string, message: string): void { + const pending = pendingByWorkspace.get(workspaceId); + const index = pending?.indexOf(message) ?? -1; + if (pending == null || index < 0) return; + pending.splice(index, 1); + if (pending.length === 0) pendingByWorkspace.delete(workspaceId); +} + +/** + * Shows the workspace's retained and later chat errors through `pushToast`, one per toast: + * `visibleToastMessage` is the input's current toast and the next error is pushed once it is gone. + * An error leaves the queue only after its toast was rendered and dismissed, so a push that never + * rendered (React batched another toast over it, or StrictMode replayed the effect) or that another + * toast replaced is pushed again. + */ +export function useChatErrorToasts( + workspaceId: string | null, + visibleToastMessage: string | null, + pushToast: (toast: { type: "error"; message: string }) => void +): void { + const pushedRef = useRef<{ message: string; displayed: boolean } | null>(null); + useEffect(() => { + if (workspaceId == null) return; + const pushed = pushedRef.current; + if (visibleToastMessage != null) { + // Dismissal is inferred from the slot clearing, so only a toast still showing this error + // counts; one that replaced it (a later success toast) means the error must show again. + if (pushed != null) pushed.displayed = pushed.message === visibleToastMessage; + return; + } + if (pushed?.displayed) dismissChatError(workspaceId, pushed.message); + pushedRef.current = null; + const showNext = () => { + if (pushedRef.current != null) return; + const message = peekChatError(workspaceId); + if (message == null) return; + pushedRef.current = { message, displayed: false }; + pushToast({ type: "error", message }); + }; + const listeners = listenersByWorkspace.get(workspaceId) ?? new Set<() => void>(); + listenersByWorkspace.set(workspaceId, listeners); + listeners.add(showNext); + showNext(); + return () => { + listeners.delete(showNext); + if (listeners.size === 0) { + listenersByWorkspace.delete(workspaceId); + } + }; + }, [workspaceId, visibleToastMessage, pushToast]); +} diff --git a/src/browser/utils/commands/sources.ts b/src/browser/utils/commands/sources.ts index 0d998a54e92..1112a6ce93b 100644 --- a/src/browser/utils/commands/sources.ts +++ b/src/browser/utils/commands/sources.ts @@ -35,6 +35,7 @@ import { import { readPersistedState, updatePersistedState } from "@/browser/hooks/usePersistedState"; import { CommandIds } from "@/browser/utils/commandIds"; import { publishAgentPluginsMutated } from "@/browser/utils/agentPluginMutations"; +import { stopStream } from "@/browser/utils/stopStream"; import { publishPluginsSectionIntent } from "@/browser/features/Settings/Sections/pluginsSectionIntents"; import { isTabType, type TabType } from "@/browser/types/rightSidebar"; import { @@ -1218,8 +1219,10 @@ export function buildCoreSources(p: BuildSourcesParams): Array<() => CommandActi if (p.selectedWorkspaceState?.awaitingUserQuestion) { return; } - await p.api?.workspace.setAutoRetryEnabled?.({ workspaceId: id, enabled: false }); - await p.api?.workspace.interruptStream({ workspaceId: id }); + if (!p.api) { + return; + } + await stopStream(p.api, id, { disableAutoRetry: true }); }, }); list.push({ diff --git a/src/browser/utils/compaction/handler.test.ts b/src/browser/utils/compaction/handler.test.ts index c4a72be3176..bafbb63ffb4 100644 --- a/src/browser/utils/compaction/handler.test.ts +++ b/src/browser/utils/compaction/handler.test.ts @@ -62,7 +62,7 @@ describe("cancelCompaction", () => { }); expect(interruptStream).toHaveBeenCalledWith({ workspaceId: "ws-1", - options: { abandonPartial: true }, + options: { abandonPartial: true, disableAutoRetry: true, retireBashMonitorAttention: true }, }); expect(calls).toEqual(["edit", "interrupt"]); }); diff --git a/src/browser/utils/compaction/handler.ts b/src/browser/utils/compaction/handler.ts index c21a4122359..9a27956cd78 100644 --- a/src/browser/utils/compaction/handler.ts +++ b/src/browser/utils/compaction/handler.ts @@ -8,6 +8,7 @@ import type { StreamingMessageAggregator } from "@/browser/utils/messages/StreamingMessageAggregator"; import { getCompactionFollowUpContent } from "@/common/types/message"; import type { APIClient } from "@/browser/contexts/API"; +import { stopStream } from "@/browser/utils/stopStream"; import { stripStagedAttachmentNotice } from "@/browser/features/ChatInput/stagedAttachments"; import { buildEditingStateFromCompaction, @@ -99,10 +100,7 @@ export async function cancelCompaction( // Interrupt stream with abandonPartial flag // Backend detects this and skips compaction (Ctrl+C flow) - await client.workspace.interruptStream({ - workspaceId, - options: { abandonPartial: true }, - }); + await stopStream(client, workspaceId, { abandonPartial: true, disableAutoRetry: true }); return true; } diff --git a/src/browser/utils/messages/applyWorkspaceChatEventToAggregator.test.ts b/src/browser/utils/messages/applyWorkspaceChatEventToAggregator.test.ts index 9877f5f4b76..cfa187aa910 100644 --- a/src/browser/utils/messages/applyWorkspaceChatEventToAggregator.test.ts +++ b/src/browser/utils/messages/applyWorkspaceChatEventToAggregator.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; -import { CUSTOM_EVENTS } from "@/common/constants/events"; +import { dismissChatError, peekChatError } from "@/browser/utils/chatErrorToasts"; import type { DeleteMessage, StreamErrorMessage, WorkspaceChatMessage } from "@/common/orpc/types"; import type { ReasoningDeltaEvent, @@ -94,40 +94,6 @@ class StubAggregator implements WorkspaceChatEventAggregator { } describe("applyWorkspaceChatEventToAggregator", () => { - function withDispatchSpy(run: (dispatched: Event[]) => T): T { - const originalWindow = globalThis.window; - const originalCustomEvent = globalThis.CustomEvent; - const dispatched: Event[] = []; - - // CI bun environment may lack CustomEvent (it was previously provided by happy-dom). - // createCustomEvent() in src/common/constants/events.ts uses `new CustomEvent(...)`. - if (typeof globalThis.CustomEvent === "undefined") { - // Minimal polyfill: only needs to carry .type and .detail for our assertions. - globalThis.CustomEvent = class CustomEvent extends Event { - detail: unknown; - - constructor(type: string, init?: CustomEventInit) { - super(type, init); - this.detail = init?.detail; - } - } as typeof globalThis.CustomEvent; - } - - globalThis.window = { - dispatchEvent: (event: Event) => { - dispatched.push(event); - return true; - }, - } as unknown as Window & typeof globalThis; - - try { - return run(dispatched); - } finally { - globalThis.window = originalWindow; - globalThis.CustomEvent = originalCustomEvent; - } - } - test("stream-start routes to handleStreamStart", () => { const aggregator = new StubAggregator(); @@ -197,29 +163,23 @@ describe("applyWorkspaceChatEventToAggregator", () => { expect(hint).toBe("immediate"); expect(aggregator.calls).toEqual(["handleRuntimeStatus:starting:ssh"]); }); - test("goal-budget-limited child events dispatch a toast without mutating messages", () => { - withDispatchSpy((dispatched) => { - const aggregator = new StubAggregator(); - const event: WorkspaceChatMessage = { - type: "goal-budget-limited", - workspaceId: "parent-1", - goalId: "goal-1", - causedByChild: true, - childWorkspaceId: "child-1", - message: "Child workspace exceeded the parent's goal budget.", - }; - - const hint = applyWorkspaceChatEventToAggregator(aggregator, event); - - expect(hint).toBe("ignored"); - expect(aggregator.calls).toEqual([]); - expect(dispatched).toHaveLength(1); - expect(dispatched[0]?.type).toBe(CUSTOM_EVENTS.GOAL_CHILD_BUDGET_TOAST); - expect((dispatched[0] as CustomEvent).detail).toEqual({ - workspaceId: "parent-1", - message: "Child workspace exceeded the parent's goal budget.", - }); - }); + test("goal-budget-limited child events publish a chat error without mutating messages", () => { + const aggregator = new StubAggregator(); + const event: WorkspaceChatMessage = { + type: "goal-budget-limited", + workspaceId: "parent-1", + goalId: "goal-1", + causedByChild: true, + childWorkspaceId: "child-1", + message: "Child workspace exceeded the parent's goal budget.", + }; + + const hint = applyWorkspaceChatEventToAggregator(aggregator, event); + + expect(hint).toBe("ignored"); + expect(aggregator.calls).toEqual([]); + expect(peekChatError("parent-1")).toBe("Child workspace exceeded the parent's goal budget."); + dismissChatError("parent-1", "Child workspace exceeded the parent's goal budget."); }); test("stream-abort clears token state before calling handleStreamAbort", () => { diff --git a/src/browser/utils/messages/applyWorkspaceChatEventToAggregator.ts b/src/browser/utils/messages/applyWorkspaceChatEventToAggregator.ts index e476b174e20..e2bc2aeb855 100644 --- a/src/browser/utils/messages/applyWorkspaceChatEventToAggregator.ts +++ b/src/browser/utils/messages/applyWorkspaceChatEventToAggregator.ts @@ -1,6 +1,7 @@ import assert from "@/common/utils/assert"; import { CUSTOM_EVENTS, createCustomEvent } from "@/common/constants/events"; import { MUX_GATEWAY_SESSION_EXPIRED_MESSAGE } from "@/common/constants/muxGatewayOAuth"; +import { publishChatError } from "@/browser/utils/chatErrorToasts"; import type { DeleteMessage, StreamErrorMessage, WorkspaceChatMessage } from "@/common/orpc/types"; import { isBashOutputEvent, @@ -94,13 +95,6 @@ function dispatchSkillsRefreshRequested(): void { window.dispatchEvent(new CustomEvent(CUSTOM_EVENTS.SKILLS_REFRESH_REQUESTED)); } -function dispatchGoalChildBudgetToast(workspaceId: string, message: string): void { - if (typeof window === "undefined") return; - window.dispatchEvent( - createCustomEvent(CUSTOM_EVENTS.GOAL_CHILD_BUDGET_TOAST, { workspaceId, message }) - ); -} - function dispatchMuxGatewaySessionExpired(): void { if (typeof window === "undefined") return; window.dispatchEvent(createCustomEvent(CUSTOM_EVENTS.MUX_GATEWAY_SESSION_EXPIRED)); @@ -206,7 +200,7 @@ export function applyWorkspaceChatEventToAggregator( if (isGoalBudgetLimitedEvent(event)) { if (allowSideEffects && event.causedByChild) { - dispatchGoalChildBudgetToast(event.workspaceId, event.message); + publishChatError(event.workspaceId, event.message); } return "ignored"; } diff --git a/src/browser/utils/stopStream.test.ts b/src/browser/utils/stopStream.test.ts new file mode 100644 index 00000000000..66485cba5db --- /dev/null +++ b/src/browser/utils/stopStream.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, test } from "bun:test"; +import type { APIClient } from "@/browser/contexts/API"; +import { dismissChatError, peekChatError } from "./chatErrorToasts"; +import { stopStream } from "./stopStream"; + +describe("stopStream", () => { + function apiReturning( + result: { success: true; data: undefined } | { success: false; error: string } + ): { api: APIClient; calls: unknown[] } { + const calls: unknown[] = []; + const api = { + workspace: { + interruptStream: (input: unknown) => { + calls.push(input); + return Promise.resolve(result); + }, + }, + } as unknown as APIClient; + return { api, calls }; + } + + test("a Stop the backend could not record is retained as the workspace's chat error", async () => { + const { api } = apiReturning({ success: false, error: "disk full" }); + + // No chat input is subscribed (the user may have switched workspaces mid-Stop): the error + // must wait for the workspace's input rather than be dropped with a one-shot event. + await stopStream(api, "ws-unrecorded"); + + expect(peekChatError("ws-unrecorded")).toBe("disk full"); + dismissChatError("ws-unrecorded", "disk full"); + expect(peekChatError("ws-unrecorded")).toBeUndefined(); + }); + + test("a Stop whose request fails in transport is retained as the workspace's chat error", async () => { + const api = { + workspace: { + interruptStream: () => Promise.reject(new Error("backend unreachable")), + }, + } as unknown as APIClient; + + await stopStream(api, "ws-transport"); + + expect(peekChatError("ws-transport")).toBe("backend unreachable"); + dismissChatError("ws-transport", "backend unreachable"); + }); + + test("a recorded Stop retires owed monitor output without a chat error", async () => { + const { api, calls } = apiReturning({ success: true, data: undefined }); + + // The retry opt-out is part of the same Stop, never a separate call ahead of it. + await stopStream(api, "ws-recorded", { abandonPartial: true, disableAutoRetry: true }); + + expect(calls).toEqual([ + { + workspaceId: "ws-recorded", + options: { abandonPartial: true, disableAutoRetry: true, retireBashMonitorAttention: true }, + }, + ]); + expect(peekChatError("ws-recorded")).toBeUndefined(); + }); +}); diff --git a/src/browser/utils/stopStream.ts b/src/browser/utils/stopStream.ts new file mode 100644 index 00000000000..4f65418833f --- /dev/null +++ b/src/browser/utils/stopStream.ts @@ -0,0 +1,29 @@ +import type { APIClient } from "@/browser/contexts/API"; +import { publishChatError } from "@/browser/utils/chatErrorToasts"; +import { getErrorMessage } from "@/common/utils/errors"; + +/** + * User Stop: interrupts the stream and dismisses owed background monitor output instead of letting + * it wake the agent. A Stop the backend could not record on disk may resume on restart, so its + * failure is shown in the workspace's chat input rather than dropped with the Result. + * + * `disableAutoRetry` rides inside the Stop rather than as a separate call: the backend persists + * the opt-out after retiring monitor attention and before acknowledging, so it can neither escape + * the Stop's durability check nor release the retry idle gate to a pending wake. + */ +export async function stopStream( + api: APIClient, + workspaceId: string, + options?: { abandonPartial?: boolean; disableAutoRetry?: boolean } +): Promise { + try { + const result = await api.workspace.interruptStream({ + workspaceId, + options: { ...options, retireBashMonitorAttention: true }, + }); + if (!result.success) publishChatError(workspaceId, result.error); + } catch (error) { + // A transport failure (backend gone mid-click) is as invisible as an Err without this. + publishChatError(workspaceId, getErrorMessage(error)); + } +} diff --git a/src/cli/run.ts b/src/cli/run.ts index 7fc1999bbb2..8d12267ac96 100644 --- a/src/cli/run.ts +++ b/src/cli/run.ts @@ -999,6 +999,20 @@ async function main(): Promise { // Budget tracking state let budgetExceeded = false; + let budgetStop: Promise | null = null; + // The budget cap is the user's Stop: go through the retiring interrupt so owed background-process + // attention is dismissed, or the after-idle reconcile would start another billed turn before + // teardown. The Err case is a stop that did not persist, not a stop that failed. The stream abort + // settles the run before retirement is durable, so teardown awaits this promise first. + const stopForBudget = (): void => { + budgetStop ??= workspaceService + .interruptStream(workspaceId, { abandonPartial: false, retireBashMonitorAttention: true }) + .then((result) => { + if (!result.success) { + log.warn("Budget stop was not recorded", { workspaceId, error: result.error }); + } + }); + }; // Centralized output type tracking for spacing type OutputType = "none" | "text" | "thinking" | "tool"; @@ -1368,7 +1382,7 @@ async function main(): Promise { const msg = `Budget exceeded ($${cost.toFixed(2)} of $${budget.toFixed(2)}) - stopping`; emitJsonLine({ type: "budget-exceeded", spent: cost, budget }); writeHumanLineClosed(`\n${chalk.yellow(msg)}`); - void session.interruptStream({ abandonPartial: false }); + stopForBudget(); } } return; @@ -1416,7 +1430,7 @@ async function main(): Promise { const msg = `Budget exceeded ($${cost.toFixed(2)} of $${budget.toFixed(2)}) - stopping`; emitJsonLine({ type: "budget-exceeded", spent: cost, budget }); writeHumanLineClosed(`\n${chalk.yellow(msg)}`); - void session.interruptStream({ abandonPartial: false }); + stopForBudget(); } } return; @@ -1570,6 +1584,7 @@ async function main(): Promise { // Contain each step, report it, and keep going. await runBestEffortCleanup( [ + { name: "budgetStop", run: () => budgetStop ?? undefined }, { name: "unsubscribe", run: () => unsubscribe() }, // Suppress monitor:stopped before session.dispose() triggers cleanup() so persisted // armed-monitor registry records survive shutdown (post-restart "monitor lost" wakes). diff --git a/src/common/constants/events.ts b/src/common/constants/events.ts index 290c0e67525..be3bf49ee1f 100644 --- a/src/common/constants/events.ts +++ b/src/common/constants/events.ts @@ -123,12 +123,6 @@ export const CUSTOM_EVENTS = { */ OPEN_GOAL_TAB: "mux:openGoalTab", - /** - * Event to show a toast when a child task pushes the parent's goal over budget. - * Detail: { workspaceId: string, message: string } - */ - GOAL_CHILD_BUDGET_TOAST: "mux:goalChildBudgetToast", - REVEAL_TIMELINE_ANCHOR: "mux:revealTimelineAnchor", /** @@ -201,10 +195,6 @@ export interface CustomEventPayloads { workspaceId: string; openCompleteInput?: boolean; }; - [CUSTOM_EVENTS.GOAL_CHILD_BUDGET_TOAST]: { - workspaceId: string; - message: string; - }; [CUSTOM_EVENTS.REVEAL_TIMELINE_ANCHOR]: { workspaceId: string; messageId?: string; diff --git a/src/common/constants/workspace.ts b/src/common/constants/workspace.ts index 7fcf9cfb94e..6fb6ac9203d 100644 --- a/src/common/constants/workspace.ts +++ b/src/common/constants/workspace.ts @@ -9,3 +9,10 @@ export const DEFAULT_RUNTIME_CONFIG: RuntimeConfig = { type: "worktree", srcBaseDir: "~/.xum/src", } as const; + +/** + * Returned by `workspace.interruptStream` for a user Stop whose stream did stop but whose startup + * abandon marker or monitor-attention retirement could not be written. + */ +export const STOP_UNRECORDED_MESSAGE = + "Stop could not be recorded on disk, so the stopped work may resume on restart."; diff --git a/src/common/orpc/schemas/api.ts b/src/common/orpc/schemas/api.ts index d688a80b800..0e6184477d8 100644 --- a/src/common/orpc/schemas/api.ts +++ b/src/common/orpc/schemas/api.ts @@ -1654,6 +1654,12 @@ export const workspace = { soft: z.boolean().optional(), abandonPartial: z.boolean().optional(), sendQueuedImmediately: z.boolean().optional(), + // User Stop only: owed bash-monitor attention is dismissed instead of waking the + // agent on the output it just stopped around. + retireBashMonitorAttention: z.boolean().optional(), + // Persist the auto-retry opt-out inside the Stop, after attention retirement is + // reserved and before the Stop is acknowledged. + disableAutoRetry: z.boolean().optional(), }) .optional(), }), diff --git a/src/common/orpc/schemas/stream.ts b/src/common/orpc/schemas/stream.ts index 2094062ffe8..271ce531407 100644 --- a/src/common/orpc/schemas/stream.ts +++ b/src/common/orpc/schemas/stream.ts @@ -336,6 +336,8 @@ export const StreamAbortEventSchema = z.object({ // Last step's provider metadata (for context window cache display) contextProviderMetadata: z.record(z.string(), z.unknown()).optional(), duration: z.number().optional(), + model: z.string().optional(), + metadataModel: z.string().optional(), }) .optional() .meta({ diff --git a/src/node/acp/agent.ts b/src/node/acp/agent.ts index ec10041fbbc..de16ec04aa0 100644 --- a/src/node/acp/agent.ts +++ b/src/node/acp/agent.ts @@ -29,6 +29,7 @@ import type { import { RequestError } from "@agentclientprotocol/sdk"; import { resolveXumEnvironmentValue } from "@/common/compat/legacyMux"; import { XUM_PRODUCT_SLUG } from "@/common/constants/product"; +import { STOP_UNRECORDED_MESSAGE } from "@/common/constants/workspace"; import { DEFAULT_COMPACTION_WORD_TARGET, WORDS_TO_TOKENS_RATIO, @@ -615,20 +616,26 @@ export class MuxAgent implements Agent { this.touchSession(sessionId); const workspaceId = this.sessionManager.getWorkspaceId(sessionId); - const interruptResult = await this.server.client.workspace.interruptStream({ workspaceId }); - - if (!interruptResult.success) { - throw new Error(`cancel: workspace.interruptStream failed: ${interruptResult.error}`); - } + const interruptResult = await this.server.client.workspace.interruptStream({ + workspaceId, + options: { retireBashMonitorAttention: true }, + }); // Resolve any pending prompt immediately after a successful interrupt request. // Backend abort events can be dropped or synthesized without a messageId when no // active stream exists; waiting exclusively for terminal chat events can leave - // ACP prompt requests hanging indefinitely. - this.resolveTurn(sessionId, { - stopReason: "cancelled", - usage: this.latestUsageBySessionId.get(sessionId), - }); + // ACP prompt requests hanging indefinitely. STOP_UNRECORDED_MESSAGE reports a stream + // that did stop (only its durable Stop records failed), so the prompt settles as + // cancelled before that failure is reported below. + if (interruptResult.success || interruptResult.error === STOP_UNRECORDED_MESSAGE) { + this.resolveTurn(sessionId, { + stopReason: "cancelled", + usage: this.latestUsageBySessionId.get(sessionId), + }); + } + if (!interruptResult.success) { + throw new Error(`cancel: workspace.interruptStream failed: ${interruptResult.error}`); + } } async setSessionConfigOption( diff --git a/src/node/services/agentSession.continuousCompaction.test.ts b/src/node/services/agentSession.continuousCompaction.test.ts index 6d442b229c6..6ec3736e495 100644 --- a/src/node/services/agentSession.continuousCompaction.test.ts +++ b/src/node/services/agentSession.continuousCompaction.test.ts @@ -718,6 +718,7 @@ describe("AgentSession continuous compaction wiring", () => { async (eventType) => { const h = await setup(); const resumed = deferred(); + const settled = deferred(); const order: string[] = []; let starts = 0; spyOn(h.aiService, "streamMessage").mockImplementation(() => { @@ -774,6 +775,11 @@ describe("AgentSession continuous compaction wiring", () => { dispatchOptions: { source: "internal-resume" }, }); order.push("apply"); + // Idle waiters (monitor wakes) must stay parked until the continuation is sent. + void h.session.waitForMidStreamCompactionSettled().then(() => { + order.push("settled"); + settled.resolve(); + }); await appendBoundary(h, followUp); return true; } @@ -819,7 +825,8 @@ describe("AgentSession continuous compaction wiring", () => { observationFinished.resolve(); } await resumed.promise; - expect(order).toEqual(["stop", "apply", "latch-released", "resume"]); + await settled.promise; + expect(order).toEqual(["stop", "apply", "latch-released", "resume", "settled"]); const history = await rows(h); expect(history.some((row) => row.metadata?.muxMetadata?.type === "compaction-request")).toBe( false diff --git a/src/node/services/agentSession.queueDispatch.test.ts b/src/node/services/agentSession.queueDispatch.test.ts index a7371ab32c5..0fdd316ca9c 100644 --- a/src/node/services/agentSession.queueDispatch.test.ts +++ b/src/node/services/agentSession.queueDispatch.test.ts @@ -1,12 +1,18 @@ import type { StreamAbortEvent } from "@/common/types/stream"; import { runSessionTerminalPolicy } from "./agentSession.testHarness"; import { describe, expect, mock, spyOn, test } from "bun:test"; +import { EventEmitter } from "node:events"; +import * as fsPromises from "node:fs/promises"; +import { createDisplayUsage } from "@/common/utils/tokens/displayUsage"; +import { getTotalCost } from "@/common/utils/tokens/usageAggregator"; import type { MuxMessageMetadata } from "@/common/types/message"; import { Err, Ok } from "@/common/types/result"; import type { WorkspaceGoalService } from "./workspaceGoalService"; import { createAgentSessionHarness, createStartedTurnHandle } from "./agentSession.testHarness"; import type { AIService } from "./aiService"; +import type { CompactionMonitor } from "./compactionMonitor"; +import type { TurnCompletion } from "./streamManager"; const TEST_MODEL = "anthropic:claude-sonnet-4-5"; const WORKSPACE_TURN_CORRELATION = { @@ -166,6 +172,76 @@ describe("AgentSession queued message tool-call dispatch", () => { } ); + test.each([ + { effectiveModel: undefined, metadataModel: undefined }, + { effectiveModel: "anthropic:claude-opus-4-1", metadataModel: undefined }, + // A Coder runtime ID has no catalog price; the request-pinned identity must price it. + { effectiveModel: "coder:acme/opus", metadataModel: "anthropic:claude-opus-4-1" }, + ])( + "accounts aborted usage against the effective model $effectiveModel priced as $metadataModel", + async ({ effectiveModel, metadataModel }) => { + const workspaceId = "abort-effective-model"; + const aiEmitter = new EventEmitter(); + const accounting = Promise.withResolvers(); + const completion = Promise.withResolvers(); + const workspaceGoalService = { + assertPricedModelForBudgetedGoal: mock(() => Promise.resolve(Ok(undefined))), + recordStreamAccounting: mock((input: { costUsd: number }) => { + accounting.resolve(input.costUsd); + return Promise.resolve(); + }), + applyPendingAfterStreamEnd: mock(() => Promise.resolve()), + requestContinuationAfterStreamEnd: mock(() => Promise.resolve()), + recordStreamStarted: mock(() => Promise.resolve()), + syncGoalModeWithChatTail: mock(() => Promise.resolve(null)), + } as unknown as WorkspaceGoalService; + const { session, cleanup } = await createAgentSessionHarness({ + workspaceId, + aiEmitter, + workspaceGoalService, + aiServiceOverrides: { + streamMessage: mock(() => { + aiEmitter.emit("stream-start", streamStartEvent(workspaceId)); + return Promise.resolve( + Ok({ messageId: "assistant-1", completion: completion.promise }) + ); + }), + }, + }); + try { + expect( + ( + await session.sendMessage( + "start", + { model: TEST_MODEL, agentId: "exec" }, + { synthetic: true, agentInitiated: true } + ) + ).success + ).toBe(true); + const usage = { inputTokens: 1_000_000, outputTokens: 0, totalTokens: 1_000_000 }; + completion.resolve({ + status: "aborted", + abortReason: "system", + streamAbort: { + type: "stream-abort", + workspaceId, + metadata: { duration: 1, usage, model: effectiveModel, metadataModel }, + }, + }); + const expectedCost = + getTotalCost( + createDisplayUsage(usage, effectiveModel ?? TEST_MODEL, undefined, metadataModel) + ) ?? 0; + expect(expectedCost).toBeGreaterThan(0); + expect(await accounting.promise).toBe(expectedCost); + await session.waitForIdle(); + } finally { + await session.dispose(); + await cleanup(); + } + } + ); + test("counts only a different direct preparing send as a superseding predecessor", async () => { const sessionHolder: { current?: { @@ -496,7 +572,11 @@ describe("AgentSession queued message tool-call dispatch", () => { session.queueMessage( "Background monitor wake", { model: TEST_MODEL, agentId: "exec", queueDispatchMode: "tool-end" }, - { synthetic: true, agentInitiated: true, cancelSignal: controller.signal } + { + synthetic: true, + agentInitiated: true, + cancelSignal: controller.signal, + } ); expect(session.hasQueuedMessages("tool-end")).toBe(true); @@ -540,7 +620,11 @@ describe("AgentSession queued message tool-call dispatch", () => { session.queueMessage( "Background monitor wake", { model: TEST_MODEL, agentId: "exec", queueDispatchMode: withdrawnMode }, - { synthetic: true, agentInitiated: true, cancelSignal: controller.signal } + { + synthetic: true, + agentInitiated: true, + cancelSignal: controller.signal, + } ); controller.abort("monitor withdrawn"); @@ -892,7 +976,13 @@ describe("AgentSession queued message tool-call dispatch", () => { test("rollback failure preserves the wake and continues acceptance", async () => { const workspaceId = "queue-dispatch-cancel-rollback-failure"; - const { session, cleanup, historyService } = await createAgentSessionHarness({ workspaceId }); + const streamMessage = mock(() => + Promise.resolve(Ok(createStartedTurnHandle(session.closingSignal))) + ); + const { session, cleanup, historyService } = await createAgentSessionHarness({ + workspaceId, + aiServiceOverrides: { streamMessage }, + }); const originalAppend = historyService.appendToHistory.bind(historyService); let markAppendStarted: () => void = () => undefined; const appendStarted = new Promise((resolve) => { @@ -926,6 +1016,7 @@ describe("AgentSession queued message tool-call dispatch", () => { agentInitiated: true, cancelState, cancelSignal: controller.signal, + withdrawAcceptedOnCancel: true, onCanceled: (reason) => { canceledReasons.push(reason); }, @@ -945,6 +1036,9 @@ describe("AgentSession queued message tool-call dispatch", () => { expect(canceledReasons).toEqual([]); expect(cancelState.canceledBeforeAcceptance).toBe(false); expect(accepted).toBe(true); + // Accepted but withdrawn: the row stays durable and no turn starts. + expect(streamMessage).not.toHaveBeenCalled(); + expect(session.isBusy()).toBe(false); const history = await historyService.getHistoryFromLatestBoundary(workspaceId); expect(history.success).toBe(true); @@ -1007,6 +1101,7 @@ describe("AgentSession queued message tool-call dispatch", () => { agentInitiated: true, cancelState, cancelSignal: controller.signal, + withdrawAcceptedOnCancel: true, onCanceled: (reason) => { canceledReasons.push(reason); }, @@ -1070,9 +1165,13 @@ describe("AgentSession queued message tool-call dispatch", () => { assertPricedModelForBudgetedGoal: mock(() => Promise.resolve(Ok(undefined))), syncGoalModeWithChatTail, } as unknown as WorkspaceGoalService; + const streamMessage = mock(() => + Promise.resolve(Ok(createStartedTurnHandle(session.closingSignal))) + ); const { session, cleanup, historyService } = await createAgentSessionHarness({ workspaceId, workspaceGoalService, + aiServiceOverrides: { streamMessage }, }); try { @@ -1088,6 +1187,7 @@ describe("AgentSession queued message tool-call dispatch", () => { agentInitiated: true, cancelState, cancelSignal: controller.signal, + withdrawAcceptedOnCancel: true, onCanceled: (reason) => { canceledReasons.push(reason); }, @@ -1107,6 +1207,102 @@ describe("AgentSession queued message tool-call dispatch", () => { expect(canceledReasons).toEqual([]); expect(cancelState.canceledBeforeAcceptance).toBe(false); expect(accepted).toBe(true); + expect(streamMessage).not.toHaveBeenCalled(); + expect(session.isBusy()).toBe(false); + + const history = await historyService.getHistoryFromLatestBoundary(workspaceId); + expect(history.success).toBe(true); + const wakeRow = history.success + ? history.data.find((message) => + message.parts.some( + (part) => part.type === "text" && part.text === "Background monitor wake" + ) + ) + : undefined; + expect(wakeRow).toBeDefined(); + + // The accepted row has no assistant follow-up, so startup recovery would otherwise treat + // it as an interrupted turn and replay the withdrawn wake. + const preferencePath = ( + session as unknown as { getAutoRetryPreferencePath: () => string } + ).getAutoRetryPreferencePath(); + const persisted = (await Bun.file(preferencePath).json()) as { + startupAutoRetryAbandon?: unknown; + }; + expect(persisted.startupAutoRetryAbandon).toEqual({ + reason: "aborted", + userMessageId: wakeRow?.id, + }); + } finally { + releaseInitialSync(); + await session.dispose(); + await cleanup(); + } + }); + + test("a wake whose admission goes stale during goal sync is finalized, not left owed", async () => { + const workspaceId = "queue-dispatch-stale-after-goal-sync"; + let markSyncStarted: () => void = () => undefined; + const syncStarted = new Promise((resolve) => { + markSyncStarted = resolve; + }); + let releaseSync: () => void = () => undefined; + const syncRelease = new Promise((resolve) => { + releaseSync = resolve; + }); + const syncGoalModeWithChatTail = mock(async () => { + markSyncStarted(); + await syncRelease; + return null; + }); + const workspaceGoalService = { + assertPricedModelForBudgetedGoal: mock(() => Promise.resolve(Ok(undefined))), + syncGoalModeWithChatTail, + } as unknown as WorkspaceGoalService; + const streamMessage = mock(() => + Promise.resolve(Ok(createStartedTurnHandle(session.closingSignal))) + ); + const { session, cleanup, historyService } = await createAgentSessionHarness({ + workspaceId, + workspaceGoalService, + aiServiceOverrides: { streamMessage }, + }); + + try { + const controller = new AbortController(); + // Stands in for the requireIdle preflight probe: a manual send enters preflight while the + // wake's durable row is already past the rollback horizon. + let manualSendInPreflight = false; + let accepted = false; + let preStreamFailures = 0; + const sendPromise = session.sendMessage( + "Background monitor wake", + { model: TEST_MODEL, agentId: "exec" }, + { + synthetic: true, + agentInitiated: true, + cancelSignal: controller.signal, + withdrawAcceptedOnCancel: true, + admissionStale: () => manualSendInPreflight, + onAccepted: () => { + accepted = true; + }, + onAcceptedPreStreamFailure: () => { + preStreamFailures += 1; + }, + } + ); + + await syncStarted; + manualSendInPreflight = true; + releaseSync(); + const result = await sendPromise; + + expect(result.success).toBe(false); + expect(accepted).toBe(true); + expect(preStreamFailures).toBe(1); + expect(streamMessage).not.toHaveBeenCalled(); + expect(session.isBusy()).toBe(false); const history = await historyService.getHistoryFromLatestBoundary(workspaceId); expect(history.success).toBe(true); @@ -1120,7 +1316,84 @@ describe("AgentSession queued message tool-call dispatch", () => { ).toBe(true); } } finally { - releaseInitialSync(); + releaseSync(); + await session.dispose(); + await cleanup(); + } + }); + + test("a wake withdrawn under on-send compaction records the persisted compaction row as abandoned", async () => { + const workspaceId = "queue-dispatch-withdrawn-compaction-row"; + let markSyncStarted: () => void = () => undefined; + const syncStarted = new Promise((resolve) => { + markSyncStarted = resolve; + }); + let releaseSync: () => void = () => undefined; + const syncRelease = new Promise((resolve) => { + releaseSync = resolve; + }); + const workspaceGoalService = { + assertPricedModelForBudgetedGoal: mock(() => Promise.resolve(Ok(undefined))), + syncGoalModeWithChatTail: mock(async () => { + markSyncStarted(); + await syncRelease; + return null; + }), + } as unknown as WorkspaceGoalService; + const streamMessage = mock(() => + Promise.resolve(Ok(createStartedTurnHandle(session.closingSignal))) + ); + const { session, cleanup, historyService } = await createAgentSessionHarness({ + workspaceId, + workspaceGoalService, + aiServiceOverrides: { streamMessage }, + }); + const internals = session as unknown as { + compactionMonitor: CompactionMonitor; + getAutoRetryPreferencePath(): string; + }; + internals.compactionMonitor = { + checkBeforeSend: () => ({ + shouldShowWarning: true, + shouldForceCompact: true, + usagePercentage: 99, + thresholdPercentage: 85, + }), + checkMidStream: () => false, + resetForNewStream: () => undefined, + setThreshold: () => undefined, + getThreshold: () => 0.85, + } as unknown as CompactionMonitor; + + try { + const controller = new AbortController(); + const sendPromise = session.sendMessage( + "Background monitor wake", + { model: TEST_MODEL, agentId: "exec" }, + { + synthetic: true, + agentInitiated: true, + cancelSignal: controller.signal, + withdrawAcceptedOnCancel: true, + } + ); + await syncStarted; + // A Stop withdraws the wake past the point of no return. + controller.abort(); + releaseSync(); + expect((await sendPromise).success).toBe(true); + expect(streamMessage).not.toHaveBeenCalled(); + + const history = await historyService.getHistoryFromLatestBoundary(workspaceId); + if (!history.success) throw new Error(history.error); + const trailing = history.data.at(-1); + expect(trailing?.metadata?.muxMetadata?.type).toBe("compaction-request"); + const persisted = JSON.parse( + await fsPromises.readFile(internals.getAutoRetryPreferencePath(), "utf-8") + ) as { startupAutoRetryAbandon?: { userMessageId?: string } }; + expect(persisted.startupAutoRetryAbandon?.userMessageId).toBe(trailing?.id); + } finally { + releaseSync(); await session.dispose(); await cleanup(); } @@ -1162,6 +1435,7 @@ describe("AgentSession queued message tool-call dispatch", () => { agentInitiated: true, cancelState, cancelSignal: controller.signal, + withdrawAcceptedOnCancel: true, onAccepted: () => { accepted = true; }, @@ -1220,6 +1494,7 @@ describe("AgentSession queued message tool-call dispatch", () => { agentInitiated: true, cancelState, cancelSignal: controller.signal, + withdrawAcceptedOnCancel: true, onCanceled: (reason) => { canceledReasons.push(reason); }, @@ -1230,6 +1505,9 @@ describe("AgentSession queued message tool-call dispatch", () => { ); await syncStarted; + // Accepted before goal sync began: a crash anywhere past the durable row leaves an accepted + // row, never one the reconciler's transcript lookup would misread as delivered. + expect(accepted).toBe(true); releaseSync(); let syncError: unknown; try { @@ -1240,7 +1518,6 @@ describe("AgentSession queued message tool-call dispatch", () => { expect(syncError).toBeInstanceOf(Error); expect((syncError as Error).message).toContain("injected goal sync failure"); - expect(accepted).toBe(true); expect(canceledReasons).toEqual([]); expect(cancelState.canceledBeforeAcceptance).toBe(false); const history = await historyService.getHistoryFromLatestBoundary(workspaceId); diff --git a/src/node/services/agentSession.startupAutoRetry.test.ts b/src/node/services/agentSession.startupAutoRetry.test.ts index 7b69288f464..287c20bc195 100644 --- a/src/node/services/agentSession.startupAutoRetry.test.ts +++ b/src/node/services/agentSession.startupAutoRetry.test.ts @@ -2,6 +2,8 @@ import type { TurnCoordinator } from "./turnCoordinator"; import { runSessionTerminalPolicy } from "./agentSession.testHarness"; import { afterEach, describe, expect, mock, spyOn, test } from "bun:test"; import { EventEmitter } from "events"; +import * as fsPromises from "fs/promises"; +import path from "path"; import { AgentSession, clearProviderConfigFixableAbandonMarkers, @@ -889,6 +891,140 @@ describe("AgentSession startup auto-retry recovery", () => { expect(events.some((event) => event.type === "auto-retry-scheduled")).toBe(false); }); + test("a marker recorded while an older clear is still unlinking is written after it and acknowledged once written", async () => { + const workspaceId = "startup-retry-serialized-abandon-writes"; + const { session, cleanup } = await createSessionBundle(workspaceId); + cleanups.push(cleanup); + + const privateSession = session as unknown as { + persistStartupAutoRetryAbandon: (reason: string, userMessageId?: string) => Promise; + clearStartupAutoRetryAbandon: () => Promise; + getAutoRetryPreferencePath: () => string; + }; + const preferencePath = privateSession.getAutoRetryPreferencePath(); + + // An in-memory preference file whose unlink and marker write the test holds open, so the clear's + // unlink and the Stop's marker write can be ordered exactly (real I/O would race them). + let fileContent: string | null = null; + let markerWrites = 0; + let holdMarkerWrites = false; + const unlinkEntered = Promise.withResolvers(); + const releaseUnlink = Promise.withResolvers(); + const releaseWrite = Promise.withResolvers(); + const macrotask = () => new Promise((resolve) => setTimeout(resolve, 0)); + const { unlink, mkdir, writeFile } = fsPromises; + const spies = [ + spyOn(fsPromises, "unlink").mockImplementation(async (target) => { + if (target !== preferencePath) return unlink(target); + unlinkEntered.resolve(); + await releaseUnlink.promise; + fileContent = null; + }), + spyOn(fsPromises, "mkdir").mockImplementation(async (target, options) => { + if (target !== path.dirname(preferencePath)) await mkdir(target, options); + }), + spyOn(fsPromises, "writeFile").mockImplementation(async (target, data, options) => { + if (target !== preferencePath || typeof data !== "string") { + return writeFile(target, data, options); + } + if (holdMarkerWrites) { + markerWrites += 1; + await releaseWrite.promise; + } + fileContent = data; + }), + ]; + try { + await privateSession.persistStartupAutoRetryAbandon("authentication", "user-1"); + holdMarkerWrites = true; + const clearing = privateSession.clearStartupAutoRetryAbandon(); + await unlinkEntered.promise; + const recording = privateSession.persistStartupAutoRetryAbandon("aborted", "user-2"); + // A macrotask drains every microtask-resolved fake step the recording could have taken: the + // marker write waits for the clear's unlink instead of racing it. + await macrotask(); + expect(markerWrites).toBe(0); + releaseUnlink.resolve(); + await clearing; + + // The clear's completion does not acknowledge the marker that is still being written. + let acknowledged: boolean | undefined; + const ack = session.recordPendingAutoRetryState().then((recorded) => { + acknowledged = recorded; + return recorded; + }); + await macrotask(); + expect(acknowledged).toBeUndefined(); + releaseWrite.resolve(); + expect(await ack).toBe(true); + await recording; + expect(fileContent).not.toBeNull(); + const persisted = JSON.parse(fileContent!) as { + startupAutoRetryAbandon?: { reason: string; userMessageId?: string }; + }; + expect(persisted.startupAutoRetryAbandon).toEqual({ + reason: "aborted", + userMessageId: "user-2", + }); + } finally { + for (const spy of spies) spy.mockRestore(); + } + }); + + test("a marker recorded while the preference file is still loading survives the load and keeps the file's opt-out", async () => { + const workspaceId = "startup-retry-marker-during-preference-load"; + const { session, cleanup } = await createSessionBundle(workspaceId); + cleanups.push(cleanup); + + const privateSession = session as unknown as { + persistStartupAutoRetryAbandon: (reason: string, userMessageId?: string) => Promise; + loadAutoRetryEnabledPreference: () => Promise; + getAutoRetryPreferencePath: () => string; + startupAutoRetryAbandon: { reason: string; userMessageId?: string } | null; + }; + const preferencePath = privateSession.getAutoRetryPreferencePath(); + await fsPromises.mkdir(path.dirname(preferencePath), { recursive: true }); + await fsPromises.writeFile(preferencePath, JSON.stringify({ enabled: false }) + "\n", "utf-8"); + + // The first preference read is held open, as in a fresh session whose startup check is still + // reading the file when a Stop withdraws an accepted wake. + const readEntered = Promise.withResolvers(); + const releaseRead = Promise.withResolvers(); + const readFile = fsPromises.readFile.bind(fsPromises); + const readSpy = spyOn(fsPromises, "readFile").mockImplementation((async ( + ...args: Parameters + ) => { + const raw = await readFile(...args); + if (args[0] !== preferencePath) return raw; + readEntered.resolve(); + await releaseRead.promise; + return raw; + }) as typeof fsPromises.readFile); + try { + const loading = privateSession.loadAutoRetryEnabledPreference(); + await readEntered.promise; + const recording = privateSession.persistStartupAutoRetryAbandon("aborted", "user-2"); + await new Promise((resolve) => setTimeout(resolve, 0)); + // Nothing is written from unloaded state while the read is pending. + expect(JSON.parse(await Bun.file(preferencePath).text())).toEqual({ enabled: false }); + releaseRead.resolve(); + expect(await loading).toBe(false); + await recording; + + expect(privateSession.startupAutoRetryAbandon).toEqual({ + reason: "aborted", + userMessageId: "user-2", + }); + expect(JSON.parse(await Bun.file(preferencePath).text())).toEqual({ + enabled: false, + startupAutoRetryAbandon: { reason: "aborted", userMessageId: "user-2" }, + }); + expect(await session.recordPendingAutoRetryState()).toBe(true); + } finally { + readSpy.mockRestore(); + } + }); + test("provider config changes preserve non-fixable abandon state without starting a stream", async () => { const workspaceId = "startup-retry-keep-abandon-on-provider-config"; const { session, aiService, events, cleanup } = await createSessionBundle(workspaceId); @@ -953,6 +1089,36 @@ describe("AgentSession startup auto-retry recovery", () => { expect(await Bun.file(preferencePath).exists()).toBe(true); }); + test("an auto-retry opt-out whose write failed is not acknowledged as recorded until it is written", async () => { + const workspaceId = "startup-retry-unrecorded-opt-out"; + const { session, cleanup } = await createSessionBundle(workspaceId); + cleanups.push(cleanup); + const preferencePath = ( + session as unknown as { getAutoRetryPreferencePath: () => string } + ).getAutoRetryPreferencePath(); + + let failWrites = true; + const { writeFile } = fsPromises; + const writeSpy = spyOn(fsPromises, "writeFile").mockImplementation( + async (target, data, options) => { + if (target === preferencePath && failWrites) throw new Error("EIO"); + return writeFile(target, data, options); + } + ); + try { + // A RetryBarrier Stop with no active stream: the opt-out is the only state it relies on. + await session.setAutoRetryEnabled(false); + expect(await Bun.file(preferencePath).exists()).toBe(false); + expect(await session.recordPendingAutoRetryState()).toBe(false); + + failWrites = false; + expect(await session.recordPendingAutoRetryState()).toBe(true); + expect(JSON.parse(await Bun.file(preferencePath).text())).toEqual({ enabled: false }); + } finally { + writeSpy.mockRestore(); + } + }); + test("provider config sweep keeps a persisted auto-retry opt-out while clearing the marker", async () => { const workspaceId = "startup-retry-sweep-keep-opt-out"; const { session, config, cleanup } = await createSessionBundle(workspaceId); diff --git a/src/node/services/agentSession.ts b/src/node/services/agentSession.ts index e0eece6a7a5..c5daa218ecc 100644 --- a/src/node/services/agentSession.ts +++ b/src/node/services/agentSession.ts @@ -236,6 +236,7 @@ import { import type { Runtime } from "@/node/runtime/Runtime"; import type { XumToolScope } from "@/common/types/toolScope"; import { execBuffered } from "@/node/utils/runtime/helpers"; +import { isErrnoWithCode } from "@/node/utils/fs"; import { renderAgentSkillSnapshotText } from "@/common/utils/agentSkills/skillSnapshot"; import type { MemorySessionContext } from "@/node/services/memoryService"; import { materializeFileAtMentions } from "@/node/services/fileAtMentions"; @@ -759,6 +760,13 @@ interface SendMessageInternalOptions { onCanceled?: (reason: string) => Promise | void; cancelState?: { canceledBeforeAcceptance: boolean }; cancelSignal?: AbortSignal; + /** + * Withdraw the send when `cancelSignal` aborts after its rows are durable but before PREPARING: + * resolve Ok without a stream and record the startup abandon marker for the row. By default a + * late abort cannot revoke an accepted send (r54). Bash-monitor wakes set this so a Stop that + * lands during acceptance or goal sync is not followed by the wake's stream. + */ + withdrawAcceptedOnCancel?: boolean; /** * For queue-dispatched sends: when the user last added to the queued * entry. Goal safety compares it against the goal's explicit @@ -944,6 +952,10 @@ export class AgentSession { private autoRetryEnabledPreference: boolean | null = null; private legacyAutoRetryEnabledHint: boolean | null = null; private startupAutoRetryAbandon: { reason: string; userMessageId?: string } | null = null; + // The preference file may not reflect memory after a failed write (see persistAutoRetryState). + private autoRetryStateUnrecorded = false; + private autoRetryStateVersion = 0; + private autoRetryStateLoad: Promise | null = null; /** Latest context-usage snapshot used for on-send compaction checks. */ private lastUsageState?: AutoCompactionUsageState; @@ -959,6 +971,7 @@ export class AgentSession { /** Prevent duplicate mid-stream compaction interrupts while we are already transitioning. */ private midStreamCompactionPending = false; + private midStreamCompactionSettledWaiters: Array<() => void> = []; private continuousCompactionAbandoned = false; private continuousCompactionStopped = false; private continuousCompactionObserving = false; @@ -1733,15 +1746,27 @@ export class AgentSession { }; } + /** + * The preference file is read once per session, and every reader and writer of the in-memory + * auto-retry state waits for that read: a load that lands late cannot overwrite a newer change, + * and a write never rebuilds the file from unloaded defaults. + */ + private loadAutoRetryState(): Promise { + this.autoRetryStateLoad ??= this.readAutoRetryState(); + return this.autoRetryStateLoad; + } + private async loadAutoRetryEnabledPreference(isCurrent = () => true): Promise { - if (this.autoRetryEnabledPreference !== null) { - return this.autoRetryEnabledPreference; - } + await this.loadAutoRetryState(); + if (this.coordinator.closing || !isCurrent()) return false; + return this.autoRetryEnabledPreference !== false; + } + private async readAutoRetryState(): Promise { const preferencePath = this.getAutoRetryPreferencePath(); try { const raw = await readFile(preferencePath, "utf-8"); - if (this.coordinator.closing || !isCurrent()) return false; + if (this.coordinator.closing) return; const parsed = JSON.parse(raw) as { enabled?: unknown; startupAutoRetryAbandon?: unknown; @@ -1753,9 +1778,8 @@ export class AgentSession { parsed.startupAutoRetryAbandon ); this.retryManager.setEnabled(enabled); - return enabled; } catch (error) { - if (this.coordinator.closing || !isCurrent()) return false; + if (this.coordinator.closing) return; // Missing preference file is the default path. Use any legacy frontend hint // (captured at onChat subscribe time) before falling back to enabled. const errno = @@ -1772,7 +1796,8 @@ export class AgentSession { if (errno === "ENOENT" && defaultEnabled === false) { // Persist migrated legacy opt-out so restart behavior no longer depends - // on renderer localStorage keys. + // on renderer localStorage keys. This write runs inside the load, so + // persistAutoRetryState must not wait for loadAutoRetryState. await this.persistAutoRetryState(); } else if (errno !== "ENOENT") { log.warn("Failed to load auto-retry preference; defaulting to enabled", { @@ -1780,15 +1805,18 @@ export class AgentSession { error: getErrorMessage(error), }); } - - return defaultEnabled; } } private autoRetryPersistence: Promise = Promise.resolve(); - private persistAutoRetryState(isCurrent = () => true): Promise { - if (!isCurrent()) return Promise.resolve(); + // Best-effort: a failed write only sets autoRetryStateUnrecorded, which the one caller that must + // not acknowledge an unrecorded write (a user Stop) checks via recordPendingAutoRetryState. Only + // the latest state change's write marks it recorded. Callers change the state only after + // loadAutoRetryState settled, so the file is never rebuilt from unloaded defaults. + private persistAutoRetryState(): Promise { + const version = ++this.autoRetryStateVersion; + this.autoRetryStateUnrecorded = true; const preferencePath = this.getAutoRetryPreferencePath(); const enabled = this.autoRetryEnabledPreference !== false; const abandon = this.startupAutoRetryAbandon; @@ -1797,6 +1825,7 @@ export class AgentSession { // must settle before a newer preference commits, or it can erase the user's opt-out. // Once memory reflects this admitted snapshot it must commit even if its generation // retires while queued; a later clear may already see null and have nothing to enqueue. + // Callers admit against the retry generation synchronously, before the load await. const payload = enabled && !abandon ? undefined @@ -1815,26 +1844,45 @@ export class AgentSession { await writeFile(preferencePath, payload, "utf-8"); } } catch (error) { - if ( - payload === undefined && - typeof error === "object" && - error !== null && - "code" in error && - error.code === "ENOENT" - ) + if (payload !== undefined || !isErrnoWithCode(error, "ENOENT")) { + log.warn("Failed to persist auto-retry preference", { + workspaceId: this.workspaceId, + error: getErrorMessage(error), + }); return; - log.warn("Failed to persist auto-retry preference", { - workspaceId: this.workspaceId, - error: getErrorMessage(error), - }); + } } + this.markAutoRetryStateRecorded(version); }) .finally(() => execution[Symbol.dispose]()); this.autoRetryPersistence = persisted; return persisted; } + private markAutoRetryStateRecorded(version: number): void { + // A state change made while this write ran has its own queued write; disk still lags memory. + if (version === this.autoRetryStateVersion) this.autoRetryStateUnrecorded = false; + } + + /** + * A user Stop is acknowledged only once the auto-retry state its stopped turn relies on is on + * disk: the startup abandon marker, or the opt-out a RetryBarrier Stop records while no stream is + * active. Otherwise the trailing row stays eligible for startup replay. A write that failed earlier + * (a withdrawn monitor wake, an aborted stream, that opt-out) is retried here, so the obligation + * survives the Stop that first reported it. The default state owes disk nothing: a file that + * outlived a failed unlink can only disable retries or suppress a replay. + */ + async recordPendingAutoRetryState(): Promise { + await this.loadAutoRetryState(); + if (this.autoRetryEnabledPreference !== false && this.startupAutoRetryAbandon === null) { + return true; + } + if (this.autoRetryStateUnrecorded) await this.persistAutoRetryState(); + return !this.autoRetryStateUnrecorded; + } + private async persistAutoRetryEnabledPreference(enabled: boolean): Promise { + await this.loadAutoRetryState(); this.autoRetryEnabledPreference = enabled; await this.persistAutoRetryState(); } @@ -1845,21 +1893,23 @@ export class AgentSession { isCurrent = () => true ): Promise { if (!isCurrent()) return; + await this.loadAutoRetryState(); this.startupAutoRetryAbandon = { reason, ...(userMessageId ? { userMessageId } : {}), }; - await this.persistAutoRetryState(isCurrent); + await this.persistAutoRetryState(); } private async clearStartupAutoRetryAbandon(isCurrent = () => true): Promise { if (!isCurrent()) return; + await this.loadAutoRetryState(); if (this.startupAutoRetryAbandon === null) { return; } this.startupAutoRetryAbandon = null; - await this.persistAutoRetryState(isCurrent); + await this.persistAutoRetryState(); } async handleProviderConfigChanged(): Promise { @@ -3403,6 +3453,7 @@ export class AgentSession { if ((internal?.preTurnMessages?.length ?? 0) > 0) internal?.onPreTurnRowsPersisted?.(); }; const accept = async (): Promise => { + if (attempt.durability === "accepted") return; await internal?.onAccepted?.(); attempt.durability = "accepted"; }; @@ -4391,18 +4442,52 @@ export class AgentSession { if (cancelSignal != null) { cancellationDisabled = true; } + // A send that opted into withdrawal and is withdrawn past the point of no return (a hard Stop + // retiring owed attention during goal sync or acceptance) keeps its durable, accepted rows but + // never streams: the Stop saw no turn to abort. The trailing UI-visible row would read as an + // interrupted turn to startup recovery, so every exit below that skips PREPARING records the + // same abandon marker a user-aborted stream leaves, before the send resolves (Stop joins the + // send for this). The withdrawal can land during any await on the way out, including + // acceptance I/O, so each exit runs this check after its last other await. + const withdrawn = () => + internal?.withdrawAcceptedOnCancel === true && cancelSignal?.aborted === true; + const abandonWithdrawnSend = async (): Promise => { + if (withdrawn()) { + // Startup recovery matches the marker against the trailing durable row, which under on-send + // compaction is the compaction request, not the never-persisted user message. + await this.updateStartupAutoRetryAbandonFromAbort( + "user", + (autoCompactionMessage ?? userMessage).id + ); + } + }; + // A stale refusal past this point keeps the durable, already accepted row, which the manual + // turn that made the admission stale consumes as context. + const refuseStaleDurableSend = async (): Promise> => { + await abandonWithdrawnSend(); + return Err(createUnknownSendMessageError(CONTEXT_MUTATION_SEND_BLOCKED_MESSAGE)); + }; // r54: the pre-turn batch is now irrevocable — rollbackPersistedTurnRows // is never invoked past this point, so even a failure in goal sync or // acceptance leaves the payload + trigger rows durable in the transcript. markRowsDurable(); + // A cancelable wake is accepted the moment its row is durable, before goal sync: its + // dispatcher treats the transcript row as proof of acceptance (a restart consumes a signal + // whose row is already there), so no later await may leave a durable, unaccepted row behind + // a crash. Startup recovery resumes the row without redelivering it, unless a Stop withdrew + // the wake. + if (cancelSignal != null) { + try { + await accept(); + } catch (error) { + await abandonWithdrawnSend(); + return Err(createUnknownSendMessageError(getErrorMessage(error))); + } + } try { await this.workspaceGoalService?.syncGoalModeWithChatTail(this.workspaceId); } catch (error) { - if (cancelSignal != null) { - // The durable row crossed the point of no return, so every later goal-sync failure must still - // finalize this monitor wake. Startup recovery can resume the row without redelivering it. - await accept(); - } + await abandonWithdrawnSend(); throw error; } @@ -4414,12 +4499,9 @@ export class AgentSession { } // Workspace may be tearing down while we await filesystem IO. - // If so, skip event emission + streaming to avoid races with dispose(). A cancelable monitor - // wake past the point of no return is already durable, so finalize it before leaving. + // If so, skip event emission + streaming to avoid races with dispose(). if (this.coordinator.disposed) { - if (cancelSignal != null && cancellationDisabled) { - await accept(); - } + await abandonWithdrawnSend(); return Ok(undefined); } @@ -4428,8 +4510,7 @@ export class AgentSession { // await, so a slider change during PREPARING (runtime warmup, model // creation) lands in the holder the stream's prepareStep will read. const turnThinkingOverride: ActiveTurnThinkingOverride = {}; - if (isAdmissionStale()) - return Err(createUnknownSendMessageError(CONTEXT_MUTATION_SEND_BLOCKED_MESSAGE)); + if (isAdmissionStale()) return refuseStaleDurableSend(); this.coordinator.acceptThinkingOverride( turnThinkingOverride, attempt.owner ?? attempt.expectedTurn @@ -4480,11 +4561,9 @@ export class AgentSession { if (isManualUserMessage) { // A fresh accepted user send supersedes any persisted startup-abandon // classification from previous turns. - if (isAdmissionStale()) - return Err(createUnknownSendMessageError(CONTEXT_MUTATION_SEND_BLOCKED_MESSAGE)); + if (isAdmissionStale()) return refuseStaleDurableSend(); await this.clearStartupAutoRetryAbandon(); - if (isAdmissionStale()) - return Err(createUnknownSendMessageError(CONTEXT_MUTATION_SEND_BLOCKED_MESSAGE)); + if (isAdmissionStale()) return refuseStaleDurableSend(); this.retryManager.cancel(); this.retryManager.setEnabled(true); await this.persistAutoRetryEnabledPreference(true); @@ -4492,8 +4571,7 @@ export class AgentSession { // Same-session retry should resume the exact accepted request we just finalized // in history, even if runtime warmup fails before streamWithHistory() starts. - if (isAdmissionStale()) - return Err(createUnknownSendMessageError(CONTEXT_MUTATION_SEND_BLOCKED_MESSAGE)); + if (isAdmissionStale()) return refuseStaleDurableSend(); this.setAutoRetryResumeState( optionsForStream, agentInitiated, @@ -4509,6 +4587,7 @@ export class AgentSession { if (this.coordinator.thinkingOverride === turnThinkingOverride) { this.coordinator.releaseThinkingOverride(turnThinkingOverride); } + await abandonWithdrawnSend(); return Err(createUnknownSendMessageError(getErrorMessage(error))); } @@ -4531,8 +4610,18 @@ export class AgentSession { // callback to revert it — returning without notifying would strand // that bookkeeping (r41). await this.settlePreparationFailure(attempt, error); + await abandonWithdrawnSend(); return Err(error); } + // A withdrawn send must not claim PREPARING (see abandonWithdrawnSend); it resolves Ok without + // a stream, like cancelBeforeAcceptance and the disposed path above. + if (withdrawn()) { + if (this.coordinator.thinkingOverride === turnThinkingOverride) { + this.coordinator.releaseThinkingOverride(turnThinkingOverride); + } + await abandonWithdrawnSend(); + return Ok(undefined); + } const preparedTurnAbortController = new AbortController(); const admission = this.coordinator.prepare( @@ -6202,7 +6291,7 @@ export class AgentSession { // Reserve through dispatch and cleanup, not just the compactor's apply latch. // Waiters/duplicate invalidations never own or clear these flags. if (this.continuousCompactionObservation === observation) { - this.midStreamCompactionPending = false; + this.settleMidStreamCompaction(); this.continuousCompactionStopped = false; this.continuousCompactionObserving = false; this.continuousCompactionObservation = null; @@ -6435,7 +6524,7 @@ export class AgentSession { } } } finally { - this.midStreamCompactionPending = false; + this.settleMidStreamCompaction(); // Preflight drains deferred to this pending compaction have no other retry: if the // compaction request never became a turn, release the queue now (no-op when it did). this.drainQueuedMessagesIfIdle(); @@ -7713,7 +7802,8 @@ export class AgentSession { const hadAnyOutput = this.activeStreamHadAnyDelta; let emittedAbort = false; try { - const activeModelForAbort = this.activeStreamContext?.modelString; + // A configured fallback can bill a different model than the requested one. + const activeModelForAbort = payload.metadata?.model ?? this.activeStreamContext?.modelString; const activeOptionsForAbort = this.activeStreamContext?.options; this.lastSystemMessageTokens = systemMessageTokens ?? this.lastSystemMessageTokens; if (activeModelForAbort) { @@ -7752,6 +7842,7 @@ export class AgentSession { model: activeModelForAbort, usage: payload.metadata?.usage, providerMetadata: payload.metadata?.providerMetadata, + metadataModel: payload.metadata?.metadataModel, goalKind: this.activeStreamContext?.goalKind, agentInitiated: this.activeStreamContext?.agentInitiated, isCompaction: hadCompactionRequest, @@ -8198,7 +8289,8 @@ export class AgentSession { ...this.getContinuousCompactionContext(context.modelString, context.options), phase: "mid-stream", }); - this.midStreamCompactionPending = false; + // The observation's finally settles the pending window only after this dispatches the + // continuation; settling earlier would let an idle waiter race the follow-up send. await this.finishContinuousCompaction(result === "applied", context); }); } catch (error) { @@ -8450,6 +8542,21 @@ export class AgentSession { return this.isBusy() || this.midStreamCompactionPending; } + /** + * Resolves once no mid-stream compaction request is pending. The window closes with no + * chat event when the compaction request never becomes a turn, so idle waiters need this + * signal rather than the stream lifecycle. + */ + waitForMidStreamCompactionSettled(): Promise { + if (!this.midStreamCompactionPending) return Promise.resolve(); + return new Promise((resolve) => this.midStreamCompactionSettledWaiters.push(resolve)); + } + + private settleMidStreamCompaction(): void { + this.midStreamCompactionPending = false; + for (const resolve of this.midStreamCompactionSettledWaiters.splice(0)) resolve(); + } + /** * Number of queued message entries (including synthetic/internal ones). The * interrupt_active archive path compares this against the delegated queued turns it is diff --git a/src/node/services/bashMonitorWakeReconciler.test.ts b/src/node/services/bashMonitorWakeReconciler.test.ts index 54a7d28ea11..d9cd3f07d8c 100644 --- a/src/node/services/bashMonitorWakeReconciler.test.ts +++ b/src/node/services/bashMonitorWakeReconciler.test.ts @@ -2,6 +2,7 @@ import * as fsPromises from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; +import type { BashMonitorWakeDisplayRecord } from "@/common/types/message"; import { classifyMachineTurnPromptKind } from "@/common/utils/machineTurnPrompts"; import type { BashMonitorRegistryRecord, @@ -12,6 +13,7 @@ import { type BashMonitorProcessSnapshot, type BashMonitorWakeDeliveryState, type BashMonitorWakeDispatch, + type BashMonitorWakeDispatchOutcome, } from "@/node/services/bashMonitorWakeReconciler"; const OWNER = "owner"; @@ -60,6 +62,8 @@ describe("BashMonitorWakeReconciler", () => { let removedOwners: string[]; let dropped: string[]; let droppedGenerations: Array; + let acknowledgeGate: ReturnType> | undefined; + let transcript: BashMonitorWakeDisplayRecord[]; let reconciler: BashMonitorWakeReconciler; beforeEach(async () => { @@ -74,6 +78,8 @@ describe("BashMonitorWakeReconciler", () => { removedOwners = []; dropped = []; droppedGenerations = []; + acknowledgeGate = undefined; + transcript = []; reconciler = new BashMonitorWakeReconciler({ sessionsDir: root, processManager: { @@ -84,6 +90,7 @@ describe("BashMonitorWakeReconciler", () => { processId, ...(matchedThroughOffset != null ? { matchedThroughOffset } : {}), }); + return acknowledgeGate?.promise; }, dropRetiredMonitor: (processId, createdAt) => { droppedGenerations.push(createdAt); @@ -107,6 +114,7 @@ describe("BashMonitorWakeReconciler", () => { }, recordTerminal: () => undefined, }, + deliveredWakes: () => Promise.resolve(transcript), onWake: (dispatch) => { dispatches.push(dispatch); return dispatchOutcome; @@ -143,10 +151,107 @@ describe("BashMonitorWakeReconciler", () => { expect(dispatches).toHaveLength(2); }); - test("superseding a queued wake uses a distinct queue key", async () => { - const queuedKeys = new Set(); - const queuedDispatches: BashMonitorWakeDispatch[] = []; - const queueing = new BashMonitorWakeReconciler({ + test("a newer match withdraws the in-flight wake and dispatches again", async () => { + live = [liveSnapshot()]; + await reconciler.reconcile(OWNER); + live = [ + liveSnapshot({ match: { throughOffset: 24, lines: ["READY again"], totalMatches: 2 } }), + ]; + + await reconciler.reconcile(OWNER); + + expect(dispatches).toHaveLength(2); + expect(dispatches[0].cancelSignal.aborted).toBe(true); + expect(dispatches[1].cancelSignal.aborted).toBe(false); + }); + + test("consumeCurrent withdraws an in-flight wake without waiting behind acceptance I/O", async () => { + live = [liveSnapshot()]; + await reconciler.reconcile(OWNER); + const wake = dispatches[0]; + + // Acceptance holds the owner lock while acknowledging the process; a Stop must still + // withdraw the wake immediately so the admission can bail before claiming a turn. + acknowledgeGate = Promise.withResolvers(); + const accepted = wake.onAccepted(); + while (acknowledged.length === 0) { + await new Promise((resolve) => setTimeout(resolve, 1)); + } + + const consumed = reconciler.consumeCurrent(OWNER); + try { + expect(wake.cancelSignal.aborted).toBe(true); + } finally { + acknowledgeGate.resolve(); + } + await accepted; + await consumed; + expect(acknowledged).toHaveLength(1); + await reconciler.reconcile(OWNER); + expect(dispatches).toHaveLength(1); + }); + + test("an accepted wake stays withdrawable until its send settles", async () => { + live = [liveSnapshot()]; + const send = Promise.withResolvers(); + const inFlight: BashMonitorWakeDispatch[] = []; + const held = new BashMonitorWakeReconciler({ + sessionsDir: root, + processManager: { + pullMonitorWakeSignals: () => live, + getMonitorWakeDeliveryState: () => Promise.resolve(deliveryState), + acknowledgeMonitorWake: (processId, _generation, matchedThroughOffset) => { + acknowledged.push({ + processId, + ...(matchedThroughOffset != null ? { matchedThroughOffset } : {}), + }); + }, + dropRetiredMonitor: () => undefined, + }, + registry: { + listAll: () => Promise.resolve(rows), + remove: () => undefined, + recordTerminal: () => undefined, + }, + deliveredWakes: () => Promise.resolve(transcript), + onWake: (dispatch) => { + inFlight.push(dispatch); + return send.promise; + }, + }); + const reconciling = held.reconcile(OWNER); + while (inFlight.length === 0) { + await new Promise((resolve) => setTimeout(resolve, 1)); + } + // The row is durable, so the wake is accepted while its send is still in preflight. + await inFlight[0].onAccepted(); + expect(acknowledged).toEqual([{ processId: "proc", matchedThroughOffset: 12 }]); + // A Stop landing before the stream starts still withdraws it. + await held.consumeCurrent(OWNER); + expect(inFlight[0].cancelSignal.aborted).toBe(true); + send.resolve("in-flight"); + await reconciling; + + live = [ + liveSnapshot({ match: { throughOffset: 24, lines: ["READY again"], totalMatches: 2 } }), + ]; + await held.reconcile(OWNER); + expect(inFlight).toHaveLength(2); + expect(inFlight[1].cancelSignal.aborted).toBe(false); + await held.dispose(OWNER); + }); + + test("a discarded process withdraws an unaccepted wake but leaves an accepted one to stream", async () => { + live = [liveSnapshot()]; + await reconciler.reconcile(OWNER); + const unaccepted = dispatches[0]; + await reconciler.discardProcess(OWNER, "proc", CREATED_AT); + expect(unaccepted.cancelSignal.aborted).toBe(true); + + // The accepted wake's send is still in flight (row durable, stream not yet started). + const send = Promise.withResolvers(); + const inFlight: BashMonitorWakeDispatch[] = []; + const held = new BashMonitorWakeReconciler({ sessionsDir: root, processManager: { pullMonitorWakeSignals: () => live, @@ -155,28 +260,153 @@ describe("BashMonitorWakeReconciler", () => { dropRetiredMonitor: () => undefined, }, registry: { - listAll: () => Promise.resolve([]), + listAll: () => Promise.resolve(rows), remove: () => undefined, recordTerminal: () => undefined, }, + deliveredWakes: () => Promise.resolve(transcript), onWake: (dispatch) => { - if (queuedKeys.has(dispatch.dedupeKey)) return "deferred"; - queuedKeys.add(dispatch.dedupeKey); - queuedDispatches.push(dispatch); - return "in-flight"; + inFlight.push(dispatch); + return send.promise; }, }); + const reconciling = held.reconcile(OWNER); + while (inFlight.length === 0) { + await new Promise((resolve) => setTimeout(resolve, 1)); + } + await inFlight[0].onAccepted(); + await held.discardProcess(OWNER, "proc", CREATED_AT); + expect(inFlight[0].cancelSignal.aborted).toBe(false); + send.resolve("in-flight"); + await reconciling; + await held.dispose(OWNER); + }); + + test("consumeCurrent withdraws the wake but keeps signals owed when the commit is refused", async () => { + live = [liveSnapshot()]; + await reconciler.reconcile(OWNER); + const wake = dispatches[0]; + + await reconciler.consumeCurrent(OWNER, () => Promise.resolve(false)); + + expect(wake.cancelSignal.aborted).toBe(true); + expect(acknowledged).toEqual([]); + await reconciler.reconcile(OWNER); + expect(dispatches).toHaveLength(2); + expect(dispatches[1].cancelSignal.aborted).toBe(false); + }); + + test("consumeCurrent retires only the attention owed when the stop was requested", async () => { live = [liveSnapshot()]; - await queueing.reconcile(OWNER); + await reconciler.reconcile(OWNER); + const stop = Promise.withResolvers(); + const stopRequested = Promise.withResolvers(); + const consuming = reconciler.consumeCurrent(OWNER, () => { + stopRequested.resolve(); + return stop.promise; + }); + await stopRequested.promise; live = [ - liveSnapshot({ match: { throughOffset: 24, lines: ["READY again"], totalMatches: 2 } }), + liveSnapshot({ match: { throughOffset: 30, lines: ["READY", "READY"], totalMatches: 2 } }), + ]; + stop.resolve(true); + await consuming; + + expect(acknowledged).toEqual([{ processId: "proc", matchedThroughOffset: 12 }]); + await reconciler.reconcile(OWNER); + expect(dispatches).toHaveLength(2); + expect(dispatches[1].cancelSignal.aborted).toBe(false); + }); + + test("consumeCurrent leaves a settlement recorded after the stop request owed", async () => { + live = [liveSnapshot()]; + await reconciler.reconcile(OWNER); + const stop = Promise.withResolvers(); + const stopRequested = Promise.withResolvers(); + const consuming = reconciler.consumeCurrent(OWNER, () => { + stopRequested.resolve(); + return stop.promise; + }); + await stopRequested.promise; + const terminal: BashMonitorTerminalSummary = { + status: "exited", + exitCode: 0, + settledAt: "2026-08-31T12:00:05.000Z", + wakeOnExit: true, + terminalStatusShown: false, + }; + live = [liveSnapshot({ terminal })]; + rows = [{ ...registryRecord(terminal), processId: "proc", taskId: "bash:proc" }]; + stop.resolve(true); + await consuming; + + await reconciler.reconcile(OWNER); + expect(dispatches).toHaveLength(2); + expect(dispatches[1].muxMetadata.records[0]).toMatchObject({ + processId: "proc", + wakeUpdatedAt: terminal.settledAt, + terminal: { status: "exited", exitCode: 0 }, + }); + }); + + test("consumeCurrent leaves a monitor failure recorded after the stop request owed", async () => { + live = [liveSnapshot()]; + await reconciler.reconcile(OWNER); + const stop = Promise.withResolvers(); + const stopRequested = Promise.withResolvers(); + const consuming = reconciler.consumeCurrent(OWNER, () => { + stopRequested.resolve(); + return stop.promise; + }); + await stopRequested.promise; + live = [liveSnapshot({ retired: true })]; + rows = [ + { + ...registryRecord(), + processId: "proc", + taskId: "bash:proc", + lost: { reason: "runtime-failure", failedAt: "2026-08-31T12:00:05.000Z" }, + }, ]; + stop.resolve(true); + await consuming; + + await reconciler.reconcile(OWNER); + expect(dispatches).toHaveLength(2); + expect(dispatches[1].muxMetadata.records[0]).toMatchObject({ + processId: "proc", + kind: "monitor-lost", + }); + }); - await queueing.reconcile(OWNER); + test("consumeCurrent keeps the registry row of a monitor armed after the stop request", async () => { + live = [liveSnapshot()]; + await reconciler.reconcile(OWNER); + const stop = Promise.withResolvers(); + const stopRequested = Promise.withResolvers(); + const consuming = reconciler.consumeCurrent(OWNER, () => { + stopRequested.resolve(); + return stop.promise; + }); + await stopRequested.promise; + const later = liveSnapshot({ + processId: "later", + taskId: "bash:later", + createdAt: "2026-08-31T12:00:05.000Z", + }); + live = [liveSnapshot(), later]; + rows = [ + { ...registryRecord(), processId: "later", taskId: "bash:later", createdAt: later.createdAt }, + ]; + stop.resolve(true); + await consuming; - expect(queuedDispatches).toHaveLength(2); - expect(queuedKeys.size).toBe(2); - expect(queuedDispatches[0].cancelSignal.aborted).toBe(true); + expect(removed).toEqual([]); + await reconciler.reconcile(OWNER); + expect(dispatches).toHaveLength(2); + expect(dispatches[1].muxMetadata.records).toEqual([ + expect.objectContaining({ processId: "later", kind: "match" }), + ]); }); test("keeps dead registry evidence until the queued wake is accepted", async () => { @@ -240,6 +470,143 @@ describe("BashMonitorWakeReconciler", () => { expect(dispatches).toHaveLength(2); }); + test("failed acceptance I/O is retried ahead of dispatch instead of redelivering the wake", async () => { + live = [liveSnapshot()]; + await reconciler.reconcile(OWNER); + const wake = dispatches[0]; + + acknowledgeGate = Promise.withResolvers(); + acknowledgeGate.promise.catch(() => undefined); + acknowledgeGate.reject(new Error("transient acknowledgement failure")); + // The accepted row is durable, so acceptance resolves and only the consumption stays owed. + await wake.onAccepted(); + await expect(reconciler.reconcile(OWNER)).rejects.toThrow("transient acknowledgement failure"); + expect(dispatches).toHaveLength(1); + + acknowledgeGate = undefined; + await reconciler.reconcile(OWNER); + expect(dispatches).toHaveLength(1); + + live = [ + liveSnapshot({ match: { throughOffset: 24, lines: ["READY again"], totalMatches: 2 } }), + ]; + await reconciler.reconcile(OWNER); + expect(dispatches).toHaveLength(2); + }); + + test("a process created in this instance never triggers a transcript scan", async () => { + let transcriptReads = 0; + const fresh = new BashMonitorWakeReconciler({ + sessionsDir: root, + processManager: { + pullMonitorWakeSignals: () => live, + getMonitorWakeDeliveryState: () => Promise.resolve(deliveryState), + acknowledgeMonitorWake: () => undefined, + dropRetiredMonitor: () => undefined, + }, + registry: { + listAll: () => Promise.resolve(rows), + remove: () => undefined, + recordTerminal: () => undefined, + }, + deliveredWakes: () => { + transcriptReads++; + return Promise.resolve(transcript); + }, + onWake: (dispatch) => { + dispatches.push(dispatch); + return "in-flight"; + }, + }); + // A failed acceptance from this instance stays owed in memory, so only processes older than + // the instance can have a delivered row the reconciler does not remember. + live = [liveSnapshot({ createdAt: new Date(Date.now() + 1_000).toISOString() })]; + await fresh.reconcile(OWNER); + expect(dispatches).toHaveLength(1); + expect(transcriptReads).toBe(0); + await fresh.dispose(OWNER); + }); + + test.each(["9", "not-a-date"])( + "a recovered process with the noncanonical age %j is still checked against the transcript", + async (createdAt) => { + // The registry accepts any createdAt string, and both sort after an ISO instance stamp: a string + // comparison would take the process for a live one and redeliver the wake its row already holds. + live = [liveSnapshot({ createdAt })]; + transcript.push({ + processId: "proc", + wakeUpdatedAt: createdAt + ":12", + kind: "match", + displayName: "CI watcher", + filter: "READY", + filterExclude: false, + }); + await reconciler.reconcile(OWNER); + expect(dispatches).toEqual([]); + expect(acknowledged).toEqual([{ processId: "proc", matchedThroughOffset: 12 }]); + } + ); + + test("a wake the transcript already carries is consumed after restart, not redelivered", async () => { + live = [liveSnapshot()]; + await reconciler.reconcile(OWNER); + // The row landed but its consumption I/O never succeeded before the app exited. + transcript.push(...dispatches[0].muxMetadata.records); + await reconciler.dispose(OWNER); + + let transcriptReads = 0; + let transcriptReadFails = true; + const afterRestart: BashMonitorWakeDispatch[] = []; + const restarted = new BashMonitorWakeReconciler({ + sessionsDir: root, + processManager: { + pullMonitorWakeSignals: () => live, + getMonitorWakeDeliveryState: () => Promise.resolve(deliveryState), + acknowledgeMonitorWake: (processId, _generation, matchedThroughOffset) => { + acknowledged.push({ + processId, + ...(matchedThroughOffset != null ? { matchedThroughOffset } : {}), + }); + }, + dropRetiredMonitor: () => undefined, + }, + registry: { + listAll: () => Promise.resolve(rows), + remove: () => undefined, + recordTerminal: () => undefined, + }, + deliveredWakes: () => { + transcriptReads++; + return transcriptReadFails + ? Promise.reject(new Error("transient transcript read")) + : Promise.resolve(transcript); + }, + onWake: (dispatch) => { + afterRestart.push(dispatch); + return "in-flight"; + }, + }); + + await expect(restarted.reconcile(OWNER)).rejects.toThrow("transient transcript read"); + expect(afterRestart).toEqual([]); + + transcriptReadFails = false; + await restarted.reconcile(OWNER); + expect(afterRestart).toEqual([]); + expect(acknowledged).toEqual([{ processId: "proc", matchedThroughOffset: 12 }]); + + live = [ + liveSnapshot({ match: { throughOffset: 24, lines: ["READY again"], totalMatches: 2 } }), + ]; + await restarted.reconcile(OWNER); + await restarted.reconcile(OWNER); + expect(afterRestart).toHaveLength(1); + expect(afterRestart[0].prompt).toContain("READY again"); + // One lookup per new outstanding key; an unchanged frontier reconciles without another read. + expect(transcriptReads).toBe(3); + await restarted.dispose(OWNER); + }); + test("full history clear consumes signals present both before and during the clear", async () => { live = [liveSnapshot()]; const token = await reconciler.beginFullHistoryClear(OWNER); @@ -315,6 +682,7 @@ describe("BashMonitorWakeReconciler", () => { }, recordTerminal: () => undefined, }, + deliveredWakes: () => Promise.resolve(transcript), onWake: (dispatch) => { restartedDispatches.push(dispatch); return "in-flight"; @@ -397,6 +765,7 @@ describe("BashMonitorWakeReconciler", () => { remove: () => undefined, recordTerminal: () => undefined, }, + deliveredWakes: () => Promise.resolve(transcript), onWake: (dispatch) => { afterRestart.push(dispatch); return "in-flight"; @@ -663,6 +1032,7 @@ describe("BashMonitorWakeReconciler", () => { }, recordTerminal: () => undefined, }, + deliveredWakes: () => Promise.resolve(transcript), onWake: (dispatch) => { afterRestart.push(dispatch); return "in-flight"; @@ -802,6 +1172,7 @@ describe("BashMonitorWakeReconciler", () => { remove: () => undefined, recordTerminal: () => undefined, }, + deliveredWakes: () => Promise.resolve(transcript), onWake: (dispatch) => { retryDispatches.push(dispatch); return "in-flight"; diff --git a/src/node/services/bashMonitorWakeReconciler.ts b/src/node/services/bashMonitorWakeReconciler.ts index 46728fc4bee..beca2c6e340 100644 --- a/src/node/services/bashMonitorWakeReconciler.ts +++ b/src/node/services/bashMonitorWakeReconciler.ts @@ -3,7 +3,7 @@ import { randomUUID } from "node:crypto"; import * as fsPromises from "node:fs/promises"; import * as path from "node:path"; -import type { MuxMessageMetadata } from "@/common/types/message"; +import type { BashMonitorWakeDisplayRecord, MuxMessageMetadata } from "@/common/types/message"; import assert from "@/common/utils/assert"; import { BASH_MONITOR_WAKE_HEADINGS } from "@/common/utils/machineTurnPrompts"; import type { @@ -11,6 +11,7 @@ import type { BashMonitorRegistryRecord, BashMonitorTerminalSummary, } from "@/node/services/bashMonitorRegistryStore"; +import { log } from "@/node/services/log"; import { MutexMap } from "@/node/utils/concurrency/mutexMap"; import { stripAnsiControlChars } from "@/node/utils/ansi"; import { isErrnoWithCode } from "@/node/utils/fs"; @@ -72,9 +73,8 @@ export type BashMonitorWakeDeliveryState = }; export interface BashMonitorWakeReconcilerProcessManager { - pullMonitorWakeSignals( - ownerWorkspaceId: string - ): Promise | readonly BashMonitorProcessSnapshot[]; + /** Synchronous so a caller can snapshot the process frontier in the tick it decides to act. */ + pullMonitorWakeSignals(ownerWorkspaceId: string): readonly BashMonitorProcessSnapshot[]; getMonitorWakeDeliveryState( processId: string, originNotAfterMs: number @@ -105,7 +105,6 @@ export interface BashMonitorWakeDispatch { ownerWorkspaceId: string; prompt: string; muxMetadata: Extract; - dedupeKey: string; cancelSignal: AbortSignal; onAccepted(): Promise; onDeferred(): Promise; @@ -155,20 +154,45 @@ interface DispatchState { signature: string; controller: AbortController; signals: readonly DerivedSignal[]; + /** Row durable and consumption at least owed. */ accepted: boolean; + /** onWake returned: the send is streaming or has exited and can no longer be withdrawn. */ + settled: boolean; } +/** Identity a persisted wake row carries per process, see buildMetadata. */ +export type DeliveredWakeRecord = Pick; + interface ReconcileState { requested: boolean; scheduled: boolean; promise?: Promise; dispatch?: DispatchState; + /** Signals of an accepted wake whose consumption I/O has not succeeded; applied before any dispatch. */ + owedAcceptance?: readonly DerivedSignal[]; + /** Frontier a committed stop still has to retire; applied before any dispatch. */ + owedRetirement?: readonly BashMonitorProcessSnapshot[]; + /** Outstanding wake keys already looked up in the transcript (see deliveredSignals). */ + transcriptChecked?: ReadonlySet; } function signalKey(processId: string, createdAt: string): string { return processId + "\u0000" + createdAt; } +/** Identifies one wake of a process; changes whenever the process has new attention to report. */ +function wakeUpdatedAt(signal: DerivedSignal): string { + return ( + signal.lost?.failedAt ?? + signal.terminal?.settledAt ?? + (signal.matchOffset != null ? signal.createdAt + ":" + signal.matchOffset : signal.createdAt) + ); +} + +function wakeKey(processId: string, updatedAt: string): string { + return processId + "\u0000" + updatedAt; +} + function normalizedTerminalStatus( terminal: BashMonitorTerminalSummary ): "exited" | "killed" | "failed" { @@ -329,12 +353,7 @@ function buildMetadata( type: "bash-monitor-wake", records: signals.map((signal) => ({ processId: signal.processId, - wakeUpdatedAt: - signal.lost?.failedAt ?? - signal.terminal?.settledAt ?? - (signal.matchOffset != null - ? signal.createdAt + ":" + signal.matchOffset - : signal.createdAt), + wakeUpdatedAt: wakeUpdatedAt(signal), kind: signal.kind === "monitor-lost" ? "monitor-lost" : "match", displayName: signal.displayName ?? signal.processId, filter: signal.filter, @@ -361,12 +380,23 @@ export class BashMonitorWakeReconciler { private readonly retryTimers = new Map(); private readonly retryAttempts = new Map(); private readonly defunctWorkspaces = new Set(); + /** Processes created before this instance can carry acceptances that lived only in a previous one. */ + private readonly constructedAtMs = Date.now(); constructor( private readonly args: { sessionsDir: string; processManager: BashMonitorWakeReconcilerProcessManager; registry: BashMonitorWakeReconcilerRegistry; + /** + * Wake identities the owner's transcript carries in rows stamped at or after `sinceMs`, the + * creation time of the oldest process being checked (-Infinity when an age is unparseable). + * A rejection holds dispatch. + */ + deliveredWakes( + ownerWorkspaceId: string, + sinceMs: number + ): Promise; onWake( dispatch: BashMonitorWakeDispatch ): Promise | BashMonitorWakeDispatchOutcome; @@ -428,10 +458,13 @@ export class BashMonitorWakeReconciler { ): Promise { await this.locks.withLock(ownerWorkspaceId, () => { const state = this.state(ownerWorkspaceId); + // An accepted wake's row is already durable; only a user Stop withdraws it (it joins the send + // and verifies the abandon marker), so a discarded process leaves it to stream. if ( - state.dispatch?.signals.some( + state.dispatch?.accepted === false && + state.dispatch.signals.some( (signal) => signal.processId === processId && signal.createdAt === createdAt - ) === true + ) ) { state.dispatch.controller.abort(); state.dispatch = undefined; @@ -440,7 +473,6 @@ export class BashMonitorWakeReconciler { }); } async beginFullHistoryClear(ownerWorkspaceId: string): Promise { - this.abortDispatch(ownerWorkspaceId); await this.consumeCurrent(ownerWorkspaceId); return { ownerWorkspaceId }; } @@ -506,22 +538,31 @@ export class BashMonitorWakeReconciler { private async reconcileOnce(ownerWorkspaceId: string): Promise { const dispatch = await this.locks.withLock(ownerWorkspaceId, async () => { + // Consumption that failed on transient I/O is retried here first (a throw lands in the + // reconcile retry backoff): an accepted wake's signals are never redelivered over the row + // the transcript already carries, and dismissed attention never dispatches when the stop's + // own idle transition reconciles. + const state = this.state(ownerWorkspaceId); + await this.acceptOwed(ownerWorkspaceId, state); + await this.retireOwed(ownerWorkspaceId, state); const collected = await this.collect(ownerWorkspaceId, true); for (const readSettled of collected.deferredReads) { void readSettled.finally(() => this.scheduleReconcile(ownerWorkspaceId)); } - await this.advanceWatermarks(ownerWorkspaceId, collected.watermarks, collected.autoConsumed); - await this.cleanup(collected.autoConsumed); + const delivered = await this.deliveredSignals(ownerWorkspaceId, state, collected.signals); + const consumed = [...collected.autoConsumed, ...delivered]; + await this.advanceWatermarks(ownerWorkspaceId, collected.watermarks, consumed); + await this.cleanup(consumed); - const state = this.state(ownerWorkspaceId); - if (collected.signals.length === 0) { + const signals = collected.signals.filter((signal) => !delivered.includes(signal)); + if (signals.length === 0) { state.dispatch?.controller.abort(); state.dispatch = undefined; return undefined; } const signature = JSON.stringify( - collected.signals.map((signal) => [ + signals.map((signal) => [ signal.key, signal.kind, signal.matchOffset, @@ -537,8 +578,9 @@ export class BashMonitorWakeReconciler { id: randomUUID(), signature, controller: new AbortController(), - signals: collected.signals, + signals, accepted: false, + settled: false, }; state.dispatch = next; return next; @@ -550,12 +592,12 @@ export class BashMonitorWakeReconciler { ownerWorkspaceId, prompt: buildPrompt(dispatch.signals), muxMetadata: buildMetadata(dispatch.signals), - dedupeKey: "bash-monitor-wake:" + ownerWorkspaceId + ":" + dispatch.id, cancelSignal: dispatch.controller.signal, onAccepted: async () => this.accept(ownerWorkspaceId, dispatch), onDeferred: async () => this.defer(ownerWorkspaceId, dispatch), }); if (outcome === "deferred") await this.defer(ownerWorkspaceId, dispatch); + else await this.settle(ownerWorkspaceId, dispatch); } catch (error) { await this.locks.withLock(ownerWorkspaceId, () => { const state = this.state(ownerWorkspaceId); @@ -573,17 +615,95 @@ export class BashMonitorWakeReconciler { return Promise.resolve(); }); } + private async settle(ownerWorkspaceId: string, dispatch: DispatchState): Promise { + await this.locks.withLock(ownerWorkspaceId, () => { + dispatch.settled = true; + this.release(ownerWorkspaceId, dispatch); + return Promise.resolve(); + }); + if (dispatch.accepted) this.scheduleReconcile(ownerWorkspaceId); + } private async accept(ownerWorkspaceId: string, dispatch: DispatchState): Promise { await this.locks.withLock(ownerWorkspaceId, async () => { if (dispatch.accepted || dispatch.controller.signal.aborted) return; dispatch.accepted = true; - const watermarks = await this.readWatermarks(ownerWorkspaceId); - await this.advanceWatermarks(ownerWorkspaceId, watermarks, dispatch.signals); - await this.cleanup(dispatch.signals); const state = this.state(ownerWorkspaceId); - if (state.dispatch === dispatch) state.dispatch = undefined; + // The accepted row is durable, so its consumption stays owed when this I/O fails: the next + // reconcile retries it ahead of any dispatch instead of failing the turn or redelivering. + state.owedAcceptance = [...(state.owedAcceptance ?? []), ...dispatch.signals]; + try { + await this.acceptOwed(ownerWorkspaceId, state); + } catch (error) { + log.warn("Bash monitor wake acceptance I/O failed; retrying before the next dispatch", { + ownerWorkspaceId, + error, + }); + } finally { + this.release(ownerWorkspaceId, dispatch); + } + }); + if (dispatch.settled) this.scheduleReconcile(ownerWorkspaceId); + } + /** + * Under the lock. An accepted wake keeps its slot until its send settles (onWake returned), so + * a Stop landing anywhere before the stream starts can still withdraw it through abortDispatch. + */ + private release(ownerWorkspaceId: string, dispatch: DispatchState): void { + const state = this.states.get(ownerWorkspaceId); + if (state?.dispatch === dispatch && dispatch.accepted && dispatch.settled) { + state.dispatch = undefined; + } + } + + private async acceptOwed(ownerWorkspaceId: string, state: ReconcileState): Promise { + if (state.owedAcceptance == null) return; + const watermarks = await this.readWatermarks(ownerWorkspaceId); + await this.advanceWatermarks(ownerWorkspaceId, watermarks, state.owedAcceptance); + await this.cleanup(state.owedAcceptance); + state.owedAcceptance = undefined; + } + + /** + * Outstanding signals whose wake row the transcript already carries. An acceptance whose + * consumption I/O kept failing until the app exited leaves the durable row as the only record + * of delivery; on the next run the signal derives as outstanding again and is consumed here + * instead of redelivered. Only processes older than this instance can be in that position (a + * failed acceptance from this instance stays owed in memory), so live monitors never trigger a + * history scan per match. Only this reconciler's own accepts add wake rows, so each outstanding + * key is looked up once and the result holds until the key leaves the outstanding set. + */ + private async deliveredSignals( + ownerWorkspaceId: string, + state: ReconcileState, + signals: readonly DerivedSignal[] + ): Promise { + const keyOf = (signal: DerivedSignal) => wakeKey(signal.processId, wakeUpdatedAt(signal)); + const checked = state.transcriptChecked ?? new Set(); + let delivered: DerivedSignal[] = []; + // Persisted ages are unvalidated strings: compare parsed times and, like startup recovery, + // count an unparseable age as recovered rather than let it sort past the instance stamp. + const createdAtMs = (signal: DerivedSignal) => Date.parse(signal.createdAt); + const recovered = signals.filter((signal) => { + const ms = createdAtMs(signal); + return !Number.isFinite(ms) || ms < this.constructedAtMs; }); - this.scheduleReconcile(ownerWorkspaceId); + if (recovered.some((signal) => !checked.has(keyOf(signal)))) { + const ages = recovered.map(createdAtMs); + const sinceMs = ages.every(Number.isFinite) ? Math.min(...ages) : -Infinity; + const rows = await this.args.deliveredWakes(ownerWorkspaceId, sinceMs); + const inTranscript = new Set( + rows.flatMap((row) => + row.processId != null && row.wakeUpdatedAt != null + ? [wakeKey(row.processId, row.wakeUpdatedAt)] + : [] + ) + ); + delivered = recovered.filter((signal) => inTranscript.has(keyOf(signal))); + } + state.transcriptChecked = new Set( + recovered.filter((signal) => !delivered.includes(signal)).map(keyOf) + ); + return delivered; } private abortDispatch(ownerWorkspaceId: string): void { @@ -592,58 +712,116 @@ export class BashMonitorWakeReconciler { state.dispatch = undefined; } - private async consumeCurrent(ownerWorkspaceId: string): Promise { - await this.locks.withLock(ownerWorkspaceId, async () => { + /** + * Withdraws the in-flight wake and consumes every signal outstanding on entry. When `commit` is + * given, the durable consumption waits for it under the lock and is skipped when it resolves + * false, leaving the withdrawn signals owed to the next reconcile. + */ + async consumeCurrent(ownerWorkspaceId: string, commit?: () => Promise): Promise { + // Withdraw before taking the lock: an acceptance in progress holds it across watermark, + // registry, and process-acknowledgement I/O, and a hard Stop must cancel the admission + // without waiting behind that. The lock slot is reserved synchronously too, ahead of any + // reconcile the stop's own stream abort triggers. + this.abortDispatch(ownerWorkspaceId); + // Snapshot the process frontier on entry as well, in the same tick: output that arrives while + // the stop waits for the lock or settles is new and stays owed to the idle agent, so the + // retirement itself can run after the commit, or on a later reconcile if its I/O fails. + const frontier = this.args.processManager.pullMonitorWakeSignals(ownerWorkspaceId); + const committed = await this.locks.withLock(ownerWorkspaceId, async () => { this.abortDispatch(ownerWorkspaceId); - const collected = await this.collect(ownerWorkspaceId, false); - const consumed = [...collected.signals, ...collected.autoConsumed]; - await this.advanceWatermarks(ownerWorkspaceId, collected.watermarks, consumed); - await this.cleanup(consumed); + if (commit != null && !(await commit())) return false; + const state = this.state(ownerWorkspaceId); + const owed = new Map( + (state.owedRetirement ?? []).map((s) => [signalKey(s.processId, s.createdAt), s] as const) + ); + for (const s of frontier) owed.set(signalKey(s.processId, s.createdAt), s); + state.owedRetirement = [...owed.values()]; + await this.retireOwed(ownerWorkspaceId, state); + return true; }); + if (!committed) this.scheduleReconcile(ownerWorkspaceId); } - private async collect( + private async retireOwed(ownerWorkspaceId: string, state: ReconcileState): Promise { + if (state.owedRetirement == null) return; + const collected = await this.collect(ownerWorkspaceId, false, state.owedRetirement); + const consumed = [...collected.signals, ...collected.autoConsumed]; + await this.advanceWatermarks(ownerWorkspaceId, collected.watermarks, consumed); + await this.cleanup(consumed); + state.owedRetirement = undefined; + } + + /** + * Live monitors merged with their registry rows, plus registry rows whose process is gone. The + * live set is pulled after the registry read so a monitor armed during the read is never taken + * for a dead row. With `asOf`, a frontier snapshotted earlier, registry state is bounded to that + * moment: terminal and lost records land only after the monitor settled or stopped in memory, so + * a snapshot without terminal was still running and one not retired had not failed, and a row + * live now but absent from the snapshot was armed since. Whatever arose since stays owed. + */ + private async candidates( ownerWorkspaceId: string, - applyFrontier: boolean - ): Promise<{ - signals: DerivedSignal[]; - autoConsumed: DerivedSignal[]; - deferredReads: Array>; - watermarks: Map; - }> { - await this.deleteLegacyWakeDirOnce(ownerWorkspaceId); - const [live, registryRows, watermarks] = await Promise.all([ - this.args.processManager.pullMonitorWakeSignals(ownerWorkspaceId), - this.args.registry.listAll(ownerWorkspaceId), - this.readWatermarks(ownerWorkspaceId), - ]); + asOf?: readonly BashMonitorProcessSnapshot[] + ): Promise> { + const registryRows = await this.args.registry.listAll(ownerWorkspaceId); + const current = this.args.processManager.pullMonitorWakeSignals(ownerWorkspaceId); + const live = asOf ?? current; const registryByKey = new Map( registryRows.map((record) => [signalKey(record.processId, record.createdAt), record] as const) ); const liveKeys = new Set( live.map((snapshot) => signalKey(snapshot.processId, snapshot.createdAt)) ); - const candidates: Array<{ snapshot: BashMonitorProcessSnapshot; deadRegistryRow: boolean }> = [ + const armedSince = new Set( + asOf == null + ? [] + : current + .map((snapshot) => signalKey(snapshot.processId, snapshot.createdAt)) + .filter((key) => !liveKeys.has(key)) + ); + return [ ...live.map((snapshot) => { const record = registryByKey.get(signalKey(snapshot.processId, snapshot.createdAt)); return { snapshot: { ...snapshot, - ...(snapshot.terminal == null && record?.terminal != null + ...(asOf == null && snapshot.terminal == null && record?.terminal != null ? { terminal: record.terminal } : {}), - ...(record?.lost != null ? { lost: record.lost } : {}), + ...(record?.lost != null && (asOf == null || snapshot.retired) + ? { lost: record.lost } + : {}), }, deadRegistryRow: false, }; }), ...registryRows - .filter((record) => !liveKeys.has(signalKey(record.processId, record.createdAt))) + .filter((record) => { + const key = signalKey(record.processId, record.createdAt); + return !liveKeys.has(key) && !armedSince.has(key); + }) .map((record) => ({ snapshot: this.fromRegistry(record, ownerWorkspaceId), deadRegistryRow: true, })), ]; + } + + private async collect( + ownerWorkspaceId: string, + applyFrontier: boolean, + liveAsOf?: readonly BashMonitorProcessSnapshot[] + ): Promise<{ + signals: DerivedSignal[]; + autoConsumed: DerivedSignal[]; + deferredReads: Array>; + watermarks: Map; + }> { + await this.deleteLegacyWakeDirOnce(ownerWorkspaceId); + const [candidates, watermarks] = await Promise.all([ + this.candidates(ownerWorkspaceId, liveAsOf), + this.readWatermarks(ownerWorkspaceId), + ]); const activeKeys = new Set( candidates.map(({ snapshot }) => signalKey(snapshot.processId, snapshot.createdAt)) ); diff --git a/src/node/services/messageQueue.test.ts b/src/node/services/messageQueue.test.ts index 48500d3cc29..a48141387ce 100644 --- a/src/node/services/messageQueue.test.ts +++ b/src/node/services/messageQueue.test.ts @@ -415,12 +415,12 @@ describe("MessageQueue", () => { expect(queue.setVisibleQueueDispatchMode("turn-end")).toBe(true); expect(queue.getVisibleQueueDispatchMode()).toBe("turn-end"); - expect(queue.getNextQueueDispatchMode()).toBe("turn-end"); + expect(queue.getNextDispatchableMode()).toBe("turn-end"); expect(queue.getQueueDispatchMode()).toBe("tool-end"); expect(queue.getMessages()).toEqual(["visible first", "visible second", "hidden wake"]); queue.dequeueNext(); - expect(queue.getNextQueueDispatchMode()).toBe("turn-end"); + expect(queue.getNextDispatchableMode()).toBe("turn-end"); }); it("reports a hidden predecessor's effective mode until the user reprioritizes the visible card", () => { @@ -440,7 +440,7 @@ describe("MessageQueue", () => { expect(queue.setVisibleQueueDispatchMode("tool-end")).toBe(true); expect(queue.getMessages()).toEqual(["visible follow-up", "hidden predecessor"]); expect(queue.getVisibleQueueDispatchMode()).toBe("tool-end"); - expect(queue.getNextQueueDispatchMode()).toBe("tool-end"); + expect(queue.getNextDispatchableMode()).toBe("tool-end"); }); it("reports the first visible entry mode instead of a later visible tool-end entry", () => { @@ -476,9 +476,9 @@ describe("MessageQueue", () => { ); expect(queue.getQueueDispatchMode()).toBe("tool-end"); - expect(queue.getNextQueueDispatchMode()).toBe("turn-end"); + expect(queue.getNextDispatchableMode()).toBe("turn-end"); queue.dequeueNext(); - expect(queue.getNextQueueDispatchMode()).toBe("tool-end"); + expect(queue.getNextDispatchableMode()).toBe("tool-end"); }); it("does not update a queue containing only hidden entries", () => { @@ -504,7 +504,7 @@ describe("MessageQueue", () => { queue.add("follow up", { ...validOptions, queueDispatchMode: "turn-end" }); expect(queue.getNextDispatchableMode()).toBe("turn-end"); - expect(queue.getNextQueueDispatchMode()).toBe("tool-end"); + expect(queue.getVisibleQueueDispatchMode()).toBe("turn-end"); }); it("should reset mode to tool-end when cleared", () => { @@ -655,6 +655,42 @@ describe("MessageQueue", () => { expect(skipped.internal?.onCanceled).toBe(peerCanceled); }); + it("ignores a withdrawn predecessor when revalidating correlations after a promotion", () => { + const turnMetadata: MuxMessageMetadata = { + type: "workspace-turn-task", + taskHandleId: "wst_parent", + ownerWorkspaceId: "grandparent", + turnId: "turn-1", + }; + const withdrawn = new AbortController(); + queue.add( + "withdrawn wake", + { ...validOptions, queueDispatchMode: "tool-end" }, + { ...hidden, cancelSignal: withdrawn.signal } + ); + withdrawn.abort(); + const peerCanceled = () => undefined; + queue.add( + "peer message", + { ...validOptions, queueDispatchMode: "turn-end", muxMetadata: turnMetadata }, + { ...hidden, workspaceTurnContinuation: true, onCanceled: peerCanceled } + ); + queue.add( + "progress report", + { ...validOptions, queueDispatchMode: "tool-end", muxMetadata: turnMetadata }, + { ...hidden, workspaceTurnContinuation: true, promoteAheadOfHiddenTurnEnd: true } + ); + + expect(queue.dequeueNext().message).toBe("withdrawn wake"); + const promoted = queue.dequeueNext(); + expect(promoted.message).toBe("progress report"); + expect(promoted.options?.muxMetadata).toEqual(turnMetadata); + const skipped = queue.dequeueNext(); + expect(skipped.message).toBe("peer message"); + expect(skipped.options?.muxMetadata).toEqual(turnMetadata); + expect(skipped.internal?.onCanceled).toBe(peerCanceled); + }); + it("ignores the hidden turn-end entries a promoted report overtakes when judging its correlation", () => { // A queued heartbeat (hidden, turn-end, uncorrelated) would make the plain check report a // superseding predecessor and strip the report's correlation before enqueue — yet the @@ -792,6 +828,55 @@ describe("MessageQueue", () => { ).toBe(false); }); + it.each(["continuation", "wake", "manual"] as const)( + "ignores withdrawn predecessors when the live successor is %s", + (kind) => { + const options = { model: "gpt-4", agentId: "exec" }; + const canceled = new AbortController(); + queue.add( + "withdrawn continuation", + { ...options, muxMetadata: metadata }, + { + synthetic: true, + cancelSignal: canceled.signal, + } + ); + queue.add( + "withdrawn wake", + { + ...options, + muxMetadata: { type: "bash-monitor-wake", records: [] }, + }, + { synthetic: true, cancelSignal: canceled.signal } + ); + canceled.abort(); + expect(queue.getNextQueueCutCandidate()).toBeUndefined(); + expect(queue.isNextEntryBashMonitorWake()).toBe(false); + expect( + queue.hasAllWorkspaceTurnContinuations("wst_followup", "parent-workspace", "turn-1") + ).toBe(true); + + const liveMetadata = + kind === "continuation" + ? metadata + : kind === "wake" + ? { type: "bash-monitor-wake" as const, records: [] } + : undefined; + queue.add("live", { ...options, muxMetadata: liveMetadata, queueDispatchMode: "turn-end" }); + expect(queue.getNextQueueCutCandidate()).toEqual({ + muxMetadata: liveMetadata, + dispatchMode: "turn-end", + }); + expect(queue.isNextEntryBashMonitorWake()).toBe(kind === "wake"); + expect( + queue.hasNextWorkspaceTurnContinuation("wst_followup", "parent-workspace", "turn-1") + ).toBe(kind === "continuation"); + expect( + queue.hasAllWorkspaceTurnContinuations("wst_followup", "parent-workspace", "turn-1") + ).toBe(kind === "continuation"); + } + ); + it("exposes the head entry's metadata and dispatch mode as the queue-cut candidate", () => { expect(queue.getNextQueueCutCandidate()).toBeUndefined(); diff --git a/src/node/services/messageQueue.ts b/src/node/services/messageQueue.ts index 3ca8a43bbb8..eeecddfab8c 100644 --- a/src/node/services/messageQueue.ts +++ b/src/node/services/messageQueue.ts @@ -283,43 +283,39 @@ export class MessageQueue { return entries.some((entry) => entry.dispatchMode === "tool-end") ? "tool-end" : "turn-end"; } - /** Dispatch boundary for the FIFO head entry — the only entry the next drain can send. */ - getNextQueueDispatchMode(): QueueDispatchMode { - return this.entries[0]?.dispatchMode ?? "tool-end"; - } - /** - * Dispatch mode of the first entry whose cancel signal has not fired, or undefined - * when none remains. Aborted entries still drain FIFO (as no-ops that fire - * onCanceled), but they are not pending work and must not arm a tool-end stop. + * The first entry whose cancel signal has not fired. Aborted entries still drain FIFO (as no-ops that fire + * onCanceled), but they are not pending work or continuations of a turn. */ + private nextDispatchableEntry(): QueueEntry | undefined { + return this.entries.find((entry) => entry.cancelSignal?.aborted !== true); + } + getNextDispatchableMode(): QueueDispatchMode | undefined { - return this.entries.find((entry) => entry.cancelSignal?.aborted !== true)?.dispatchMode; + return this.nextDispatchableEntry()?.dispatchMode; } /** - * Whether every queued entry continues the exact workspace turn correlation. + * Whether every pending queued entry continues the exact workspace turn correlation. * * The caller uses this for a new continuation that has not entered the queue. - * An unrelated entry anywhere ahead of it supersedes the correlation. + * An unrelated pending entry anywhere ahead of it supersedes the correlation. */ hasAllWorkspaceTurnContinuations( taskHandleId: string, ownerWorkspaceId: string, turnId: string ): boolean { - return ( - this.entries.length > 0 && - this.entries.every((entry) => { - const metadata = entry.muxMetadata; - return ( - isWorkspaceTurnMetadata(metadata) && - metadata.taskHandleId === taskHandleId && - metadata.ownerWorkspaceId === ownerWorkspaceId && - metadata.turnId === turnId - ); - }) - ); + return this.entries.every((entry) => { + if (entry.cancelSignal?.aborted === true) return true; + const metadata = entry.muxMetadata; + return ( + isWorkspaceTurnMetadata(metadata) && + metadata.taskHandleId === taskHandleId && + metadata.ownerWorkspaceId === ownerWorkspaceId && + metadata.turnId === turnId + ); + }); } /** @@ -336,6 +332,7 @@ export class MessageQueue { turnId: string ): boolean { return this.entries.slice(0, this.trailingHiddenTurnEndRunStart()).every((entry) => { + if (entry.cancelSignal?.aborted === true) return true; const metadata = entry.muxMetadata; return ( isWorkspaceTurnMetadata(metadata) && @@ -364,14 +361,14 @@ export class MessageQueue { } /** - * Whether the next entry continues the exact workspace turn correlation. + * Whether the next dispatchable entry continues the exact workspace turn correlation. */ hasNextWorkspaceTurnContinuation( taskHandleId: string, ownerWorkspaceId: string, turnId: string ): boolean { - const metadata = this.entries[0]?.muxMetadata; + const metadata = this.nextDispatchableEntry()?.muxMetadata; return ( isWorkspaceTurnMetadata(metadata) && metadata.taskHandleId === taskHandleId && @@ -381,7 +378,7 @@ export class MessageQueue { } /** - * FIFO head entry's cut-attribution view: its first muxMetadata plus dispatch mode. + * Next dispatchable entry's cut-attribution view: its first muxMetadata plus dispatch mode. * * Soundness of metadata-based cut attribution rests on the sealing invariant * (see class docblock): workspace-turn entries are sealed at add time and @@ -392,7 +389,7 @@ export class MessageQueue { getNextQueueCutCandidate(): | { muxMetadata: unknown; dispatchMode: QueueDispatchMode } | undefined { - const head = this.entries[0]; + const head = this.nextDispatchableEntry(); if (head == null) { return undefined; } @@ -400,13 +397,10 @@ export class MessageQueue { } /** - * Whether the next entry to dispatch is a bash-monitor wake. Wake sends are - * the only queued input that continues an open delegated workspace turn - * (see AgentSession.inheritOpenWorkspaceTurnMetadata); any other head entry - * supersedes the turn when it dispatches. + * Bash-monitor wakes inherit an open delegated turn's correlation at dispatch. */ isNextEntryBashMonitorWake(): boolean { - const muxMetadata = this.entries[0]?.muxMetadata; + const muxMetadata = this.nextDispatchableEntry()?.muxMetadata; if (typeof muxMetadata !== "object" || muxMetadata === null) return false; return (muxMetadata as Record).type === "bash-monitor-wake"; } @@ -423,9 +417,13 @@ export class MessageQueue { /** * Dispatch mode for user-visible entries only. Backend-initiated maintenance/wake * messages should not change the queue badge shown beside the user's own follow-up. + * Derived from the entry the next drain actually sends, so a withdrawn head cannot show a + * boundary the live message will not dispatch at. */ getVisibleQueueDispatchMode(): QueueDispatchMode { - return this.getVisibleEntries().length > 0 ? this.getNextQueueDispatchMode() : "tool-end"; + return this.getVisibleEntries().length > 0 + ? (this.getNextDispatchableMode() ?? "tool-end") + : "tool-end"; } /** @@ -440,6 +438,9 @@ export class MessageQueue { let priorCorrelation: WorkspaceTurnMetadata | undefined; for (const entry of this.entries) { + // Withdrawn entries drain as no-ops: neither predecessors nor correlation holders, as in + // hasAllWorkspaceTurnContinuations. + if (entry.cancelSignal?.aborted === true) continue; const metadata = isWorkspaceTurnMetadata(entry.muxMetadata) ? entry.muxMetadata : undefined; const matchesPriorCorrelation = metadata != null && diff --git a/src/node/services/streamManager.test.ts b/src/node/services/streamManager.test.ts index 237d2e73869..097938ba782 100644 --- a/src/node/services/streamManager.test.ts +++ b/src/node/services/streamManager.test.ts @@ -5,7 +5,11 @@ import * as path from "node:path"; import { KNOWN_MODELS } from "@/common/constants/knownModels"; import type { ProvidersConfigMap } from "@/common/orpc/types"; -import { StreamEndEventSchema, ToolCallStartEventSchema } from "@/common/orpc/schemas/stream"; +import { + StreamAbortEventSchema, + StreamEndEventSchema, + ToolCallStartEventSchema, +} from "@/common/orpc/schemas/stream"; import type { CompletedMessagePart, ToolCallEndEvent, @@ -6814,6 +6818,32 @@ describe("StreamManager - aborted stream usage persistence", () => { }); }); + test("emits the effective fallback model and its pinned pricing identity with aborted usage", async () => { + const streamManager = new StreamManager(historyService); + const effectiveModel = "coder:acme/opus"; + const pinnedMetadataModel = "anthropic:claude-opus-4-1"; + const abort = Promise.withResolvers(); + onTurnEngineEvent(streamManager, "stream-abort", (event) => abort.resolve(event)); + const cleanupAborted = getPrivateMethodForTests( + streamManager, + "cleanupAbortedStream" + ); + await cleanupAborted.call( + streamManager, + "fallback-abort", + { + ...createAbortStreamInfo("fallback-message"), + model: effectiveModel, + metadataModel: pinnedMetadataModel, + }, + "system" + ); + const event = StreamAbortEventSchema.parse(await abort.promise); + expect(event.metadata?.model).toBe(effectiveModel); + expect(event.metadata?.metadataModel).toBe(pinnedMetadataModel); + expect(event.metadata?.usage?.inputTokens).toBe(120); + }); + test.each(["commit-err", "commit-throw", "delete-err", "delete-throw"] as const)( "abort settles once and retains recovery data on %s", async (failure) => { diff --git a/src/node/services/streamManager.ts b/src/node/services/streamManager.ts index 9e2d078e9aa..edc09d6ee46 100644 --- a/src/node/services/streamManager.ts +++ b/src/node/services/streamManager.ts @@ -2035,7 +2035,15 @@ export class StreamManager { type: "stream-abort", workspaceId, messageId: streamInfo.messageId, - metadata: { usage, contextUsage, duration, providerMetadata, contextProviderMetadata }, + metadata: { + usage, + contextUsage, + duration, + providerMetadata, + contextProviderMetadata, + model: streamInfo.model, + metadataModel: streamInfo.metadataModel, + }, abortReason, abandonPartial, acpPromptId: streamInfo.initialMetadata?.acpPromptId, diff --git a/src/node/services/taskWorkspaceSeam.ts b/src/node/services/taskWorkspaceSeam.ts index 72575991977..5d98fd0ae83 100644 --- a/src/node/services/taskWorkspaceSeam.ts +++ b/src/node/services/taskWorkspaceSeam.ts @@ -327,6 +327,8 @@ export interface SendMessageInternalOptions { cancelState?: { canceledBeforeAcceptance: boolean }; /** Cancels a synthetic send even after it has left MessageQueue for PREPARING. */ cancelSignal?: AbortSignal; + /** Let a late `cancelSignal` abort withdraw the send after its rows are durable (see AgentSession). */ + withdrawAcceptedOnCancel?: boolean; /** * Synchronous staleness probe from the caller, re-evaluated at the real admission points * (the enqueue block and the session's turn-admission gates) in addition to the diff --git a/src/node/services/workspaceService.test.ts b/src/node/services/workspaceService.test.ts index d1c05baf95f..e1b4d86f8e1 100644 --- a/src/node/services/workspaceService.test.ts +++ b/src/node/services/workspaceService.test.ts @@ -3,6 +3,7 @@ import type { TurnCoordinator } from "./turnCoordinator"; import { MutexMap } from "@/node/utils/concurrency/mutexMap"; import { describe, expect, test, mock, beforeEach, afterEach, spyOn, type Mock } from "bun:test"; import { WorkspaceService, generateForkBranchName, generateForkTitle } from "./workspaceService"; +import { STOP_UNRECORDED_MESSAGE } from "@/common/constants/workspace"; import { registerInProcessWorkflowRun } from "@/node/services/workflows/workflowArchiveAdmission"; import type { IdleCompactionOutcome } from "./idleCompactionService"; import type { AgentSession } from "./agentSession"; @@ -32,6 +33,9 @@ import type { SessionTimingService } from "./sessionTimingService"; import { SessionUsageService } from "./sessionUsageService"; import type { AIService } from "./aiService"; import { MockLanguageModelV3, simulateReadableStream } from "ai/test"; +import { streamText, tool } from "ai"; +import { z } from "zod"; +import { StreamManager } from "./streamManager"; import type { LanguageModelV3StreamPart } from "@ai-sdk/provider"; import { EXPERIMENT_IDS } from "@/common/constants/experiments"; import type { ExperimentsService } from "./experimentsService"; @@ -57,7 +61,7 @@ import type { DesktopSessionManager } from "@/node/services/desktop/DesktopSessi import type { WorktreeArchiveSnapshot } from "@/common/schemas/project"; import type { BashToolResult } from "@/common/types/tools"; import type { SendMessageOptions, WorkspaceChatMessage } from "@/common/orpc/types"; -import { createMuxMessage } from "@/common/types/message"; +import { createMuxMessage, type MuxMessageMetadata } from "@/common/types/message"; import { buildStagedAttachmentNotice } from "@/browser/features/ChatInput/stagedAttachments"; import { WORKFLOW_RESULT_METADATA_TYPE, @@ -89,6 +93,13 @@ import { // nit DEREM-50) — import instead of defining local copies. import { drainPendingDispatches, waitForCondition } from "./testDispatchHelpers"; import { sandboxHostService } from "./sandbox/sandboxHostService"; +import type { + BashMonitorProcessSnapshot, + BashMonitorWakeReconciler, + BashMonitorWakeReconcilerProcessManager, + BashMonitorWakeReconcilerRegistry, + BashMonitorWakeDispatch, +} from "./bashMonitorWakeReconciler"; // Helper to access private renamingWorkspaces set function addToRenamingWorkspaces(service: WorkspaceService, workspaceId: string): void { @@ -235,9 +246,10 @@ describe("WorkspaceService bash monitor wake reconciler wiring", () => { const { config, historyService, cleanup } = await createTestHistoryService(); const events = new EventEmitter(); const backgroundProcessManager = Object.assign(events, { + cleanup: mock(() => Promise.resolve()), notifyMonitorWakeStateChanged: mock(() => undefined), getActiveMonitorCount: mock(() => 0), - pullMonitorWakeSignals: mock(() => Promise.resolve([])), + pullMonitorWakeSignals: mock(() => []), getMonitorWakeDeliveryState: mock(() => Promise.resolve(undefined)), acknowledgeMonitorWake: mock(() => undefined), dropRetiredMonitor: mock(() => undefined), @@ -252,9 +264,1020 @@ describe("WorkspaceService bash monitor wake reconciler wiring", () => { ), backgroundProcessManager, }); - return { config, service, events, cleanup }; + return { config, historyService, backgroundProcessManager, service, events, cleanup }; } + async function createActiveWakeHarness(options?: { + workspaceGoalService?: WorkspaceGoalService; + }) { + const fixture = await createWakeWiringService(); + const { config, service, historyService, backgroundProcessManager } = fixture; + const workspaceId = "monitor-attention-owner"; + await config.addWorkspace("/tmp/monitor-attention-project", { + id: workspaceId, + name: workspaceId, + projectName: "monitor-attention-project", + projectPath: "/tmp/monitor-attention-project", + runtimeConfig: { type: "local" }, + }); + const model = "anthropic:claude-sonnet-4-5"; + const aiEmitter = new EventEmitter(); + const requests: Array[0]> = []; + const completions: Array>> = []; + const launched = new EventEmitter(); + let streaming = false; + const harness = await createAgentSessionHarness({ + workspaceId, + config, + historyService, + backgroundProcessManager, + aiEmitter, + workspaceGoalService: options?.workspaceGoalService, + aiServiceOverrides: { + isStreaming: () => streaming, + streamMessage: mock((request: Parameters[0]) => { + requests.push(request); + const completion = Promise.withResolvers(); + completions.push(completion); + // Session shutdown retires an in-flight handle (as createStartedTurnHandle does), so + // finish() can drain a turn the test never completed. + harness.session.closingSignal.addEventListener( + "abort", + () => completion.resolve({ status: "aborted", abortReason: "user" }), + { once: true } + ); + streaming = true; + const messageId = "assistant-" + requests.length; + aiEmitter.emit("stream-start", { + type: "stream-start", + workspaceId, + messageId, + model, + startTime: Date.now(), + }); + launched.emit("start"); + return Promise.resolve(Ok({ messageId, completion: completion.promise })); + }), + }, + }); + const internal = service as unknown as { + aiService: typeof harness.aiService; + sessions: Map; + bashMonitorRecoveryPromise: Promise; + bashMonitorWakeReconciler: BashMonitorWakeReconciler; + pendingBashMonitorWakeIdleWaitsByOwner: Map>; + getDelegatedTurnContinuationSendOptions(workspaceId: string): Promise; + dispatchBashMonitorWake(dispatch: BashMonitorWakeDispatch): Promise<"in-flight" | "deferred">; + }; + await internal.bashMonitorRecoveryPromise; + internal.aiService = harness.aiService; + internal.sessions.set(workspaceId, harness.session); + internal.getDelegatedTurnContinuationSendOptions = () => + Promise.resolve({ model, agentId: "exec" }); + const signals: BashMonitorProcessSnapshot[] = []; + let shown = 0; + spyOn(backgroundProcessManager, "pullMonitorWakeSignals").mockImplementation(() => [ + ...signals, + ]); + spyOn(backgroundProcessManager, "getMonitorWakeDeliveryState").mockImplementation(() => + Promise.resolve({ status: "settled", shownThroughOffset: shown, terminalStatusShown: false }) + ); + const reconciler = internal.bashMonitorWakeReconciler; + const dispatch = spyOn(internal, "dispatchBashMonitorWake"); + const complete = async (finishReason = "stop") => { + const messageId = "assistant-" + requests.length; + const message = createMuxMessage(messageId, "assistant", "final answer", { + model, + finishReason, + muxMetadata: requests[requests.length - 1].muxMetadata, + }); + await historyService.appendToHistory(workspaceId, message); + const completed = new Promise((resolve) => { + const unsubscribe = harness.session.onChatEvent(({ message: event }) => { + if (event.type === "stream-end") { + unsubscribe(); + resolve(); + } + }); + }); + streaming = false; + const streamEnd = { + type: "stream-end" as const, + workspaceId, + parts: [{ type: "text" as const, text: "final answer" }], + metadata: { model, finishReason }, + }; + aiEmitter.emit("stream-end", { ...streamEnd, messageId }); + completions[requests.length - 1].resolve({ status: "completed", streamEnd }); + await completed; + }; + const abort = (abortReason: "user" | "system") => { + const messageId = "assistant-" + requests.length; + const streamAbort = { type: "stream-abort" as const, workspaceId, metadata: { duration: 1 } }; + streaming = false; + aiEmitter.emit("stream-abort", { ...streamAbort, messageId, abortReason }); + completions[requests.length - 1].resolve({ status: "aborted", abortReason, streamAbort }); + }; + return { + ...fixture, + ...harness, + workspaceId, + model, + requests, + launched, + internal, + reconciler, + dispatch, + complete, + abort, + stopStream: spyOn(harness.aiService, "stopStream"), + addAttention: async (offset: number) => { + signals.splice( + 0, + signals.length, + ...["first", "second"].map((processId) => ({ + processId, + taskId: "bash:" + processId, + ownerWorkspaceId: workspaceId, + filter: "READY", + filterExclude: false, + script: "watch", + createdAt: "2026-01-01T00:00:00.000Z", + retired: false, + match: { throughOffset: offset, lines: ["READY " + offset], totalMatches: 1 }, + })) + ); + fixture.events.emit("monitor:match", workspaceId, {}); + await reconciler.reconcile(workspaceId); + }, + consume: async (offset: number) => { + shown = offset; + fixture.events.emit("output:shown", workspaceId, {}); + await reconciler.reconcile(workspaceId); + }, + finish: async () => { + await reconciler.dispose(workspaceId); + await harness.session.dispose(); + await fixture.cleanup(); + }, + }; + } + + test("a wake row already in history is consumed without a dispatch, even behind a compaction boundary", async () => { + const h = await createActiveWakeHarness(); + const acknowledged = spyOn(h.backgroundProcessManager, "acknowledgeMonitorWake"); + try { + // The row is durable but its acceptance never reached the watermark (I/O failed until exit); + // a later compaction moved it out of the window the model sees. + await h.historyService.appendToHistory( + h.workspaceId, + createMuxMessage("wake-delivered", "user", "Monitor matched", { + timestamp: Date.now(), + muxMetadata: { + type: "bash-monitor-wake", + records: ["first", "second"].map((processId) => ({ + processId, + wakeUpdatedAt: "2026-01-01T00:00:00.000Z:7", + kind: "match" as const, + displayName: processId, + filter: "READY", + filterExclude: false, + })), + }, + }) + ); + await h.historyService.appendToHistory( + h.workspaceId, + createMuxMessage("summary-1", "assistant", "Summary", { + timestamp: Date.now(), + compactionBoundary: true, + compacted: true, + compactionEpoch: 1, + muxMetadata: { type: "compaction-summary" }, + }) + ); + await h.addAttention(7); + expect(h.dispatch).not.toHaveBeenCalled(); + expect(acknowledged).toHaveBeenCalledTimes(2); + await h.addAttention(12); + expect(h.dispatch).toHaveBeenCalledTimes(1); + } finally { + await h.finish(); + } + }); + + test("a wake persisted as an on-send compaction request is consumed without a dispatch", async () => { + const h = await createActiveWakeHarness(); + const acknowledged = spyOn(h.backgroundProcessManager, "acknowledgeMonitorWake"); + try { + await h.historyService.appendToHistory( + h.workspaceId, + createMuxMessage("wake-compaction", "user", "/compact", { + timestamp: Date.now(), + muxMetadata: { + type: "compaction-request", + rawCommand: "/compact", + parsed: { + followUpContent: { + text: "Monitor matched", + model: h.model, + agentId: "exec", + muxMetadata: { + type: "bash-monitor-wake", + records: ["first", "second"].map((processId) => ({ + processId, + wakeUpdatedAt: "2026-01-01T00:00:00.000Z:7", + kind: "match" as const, + displayName: processId, + filter: "READY", + filterExclude: false, + })), + }, + }, + }, + }, + }) + ); + await h.addAttention(7); + expect(h.dispatch).not.toHaveBeenCalled(); + expect(acknowledged).toHaveBeenCalledTimes(2); + } finally { + await h.finish(); + } + }); + + test("an archived owner's wake is held without an idle-retry loop and dispatches on unarchive", async () => { + const h = await createActiveWakeHarness(); + const send = spyOn(h.service, "sendMessage"); + try { + await h.config.editConfig((config) => { + for (const project of config.projects.values()) { + for (const workspace of project.workspaces) { + if (workspace.id === h.workspaceId) workspace.archivedAt = new Date().toISOString(); + } + } + return config; + }); + await h.addAttention(7); + expect(send).not.toHaveBeenCalled(); + expect(h.internal.pendingBashMonitorWakeIdleWaitsByOwner.has(h.workspaceId)).toBe(false); + expect((await h.reconciler.snapshot(h.workspaceId)).pendingWakeKinds.size).toBe(2); + + // An unarchive whose restoration fails rolls back to archived; the wake must not have run + // against the half-restored checkout in between. + const snapshots = h.internal as unknown as { + worktreeArchiveSnapshotService?: { restoreSnapshotAfterUnarchive(): Promise }; + }; + snapshots.worktreeArchiveSnapshotService = { + restoreSnapshotAfterUnarchive: () => Promise.resolve(Err("restore failed")), + }; + expect((await h.service.unarchive(h.workspaceId)).success).toBe(false); + await h.reconciler.reconcile(h.workspaceId); + expect(send).not.toHaveBeenCalled(); + + snapshots.worktreeArchiveSnapshotService = undefined; + // Restoration succeeds but a follow-up step throws: unarchivedAt is already persisted and a + // retried unarchive would not run the hooks again, so the held attention must still wake. + spyOn( + h.internal as unknown as { syncCodeWorkspaceFiles(): Promise }, + "syncCodeWorkspaceFiles" + ).mockImplementationOnce(() => { + throw new Error("sync failed"); + }); + expect((await h.service.unarchive(h.workspaceId)).success).toBe(false); + // Unarchive itself schedules the reconcile; no manual reconcile here. + await waitForCondition(() => h.requests.length === 1); + } finally { + await h.finish(); + } + }); + + test("malformed wake metadata in history neither stalls nor consumes an outstanding wake", async () => { + const h = await createActiveWakeHarness(); + try { + await h.historyService.appendToHistory( + h.workspaceId, + createMuxMessage("wake-corrupt", "user", "Monitor matched", { + timestamp: Date.now(), + muxMetadata: { + type: "bash-monitor-wake", + records: [null, "junk", { processId: "first" }], + } as unknown as MuxMessageMetadata, + }) + ); + await h.addAttention(7); + expect(h.dispatch).toHaveBeenCalledTimes(1); + } finally { + await h.finish(); + } + }); + + test("the SDK answers in the original stream after repeated owed wakes are consumed", async () => { + const h = await createActiveWakeHarness(); + let step = 0; + let offset = 0; + const sdkModel = new MockLanguageModelV3({ + doStream: () => { + step++; + const chunks: LanguageModelV3StreamPart[] = + step <= 6 + ? [ + { + type: "tool-call", + toolCallId: "tool-" + step, + toolName: step % 2 === 1 ? "held_tool" : "task_await", + input: "{}", + }, + ] + : [ + { type: "text-start", id: "answer" }, + { type: "text-delta", id: "answer", delta: "final answer" }, + { type: "text-end", id: "answer" }, + ]; + chunks.push({ + type: "finish", + finishReason: { unified: step <= 6 ? "tool-calls" : "stop", raw: undefined }, + usage: { + inputTokens: { total: 1, noCache: 1, cacheRead: 0, cacheWrite: 0 }, + outputTokens: { total: 1, text: 1, reasoning: 0 }, + }, + }); + return Promise.resolve({ + stream: simulateReadableStream({ chunks, initialDelayInMs: null, chunkDelayInMs: null }), + }); + }, + }); + try { + await h.session.sendMessage("original", { model: h.model, agentId: "exec" }); + const engine = new StreamManager(h.historyService) as unknown as { + createStopWhenCondition( + request: Pick[0], "hasQueuedMessages"> + ): Array<(options: { steps: unknown[] }) => boolean>; + }; + const result = streamText({ + model: sdkModel, + prompt: "run the monitored tasks", + stopWhen: engine.createStopWhenCondition(h.requests[0]), + tools: { + held_tool: tool({ + inputSchema: z.object({}), + execute: async () => { + offset += 10; + await h.addAttention(offset); + expect((await h.reconciler.snapshot(h.workspaceId)).pendingWakeKinds.size).toBe(2); + return "foreground finished"; + }, + }), + task_await: tool({ + inputSchema: z.object({}), + execute: async () => { + await h.consume(offset); + return "READY"; + }, + }), + }, + }); + expect(await result.text).toBe("final answer"); + expect(step).toBe(7); + expect(h.dispatch).toHaveBeenCalled(); + await h.complete(); + await h.internal.pendingBashMonitorWakeIdleWaitsByOwner.get(h.workspaceId); + await h.reconciler.reconcile(h.workspaceId); + expect(h.requests).toHaveLength(1); + const history = await h.historyService.getHistoryFromLatestBoundary(h.workspaceId); + expect(history.success && history.data.map((row) => row.role)).toEqual(["user", "assistant"]); + } finally { + await h.finish(); + } + }); + + test.each([false, true])( + "owed monitor attention never cuts an active tool (native=%s)", + async (providerExecuted) => { + const h = await createActiveWakeHarness(); + try { + expect( + (await h.session.sendMessage("original", { model: h.model, agentId: "exec" })).success + ).toBe(true); + for (const offset of [10, 20, 30]) { + h.aiEmitter.emit("tool-call-start", { + type: "tool-call-start", + workspaceId: h.workspaceId, + messageId: "assistant-1", + toolCallId: "held-tool", + toolName: "bash", + args: {}, + timestamp: Date.now(), + }); + await h.addAttention(offset); + expect((await h.reconciler.snapshot(h.workspaceId)).pendingWakeKinds.size).toBe(2); + expect(h.dispatch).toHaveBeenCalled(); + expect(h.requests[0].hasQueuedMessages?.("tool-end")).toBe(false); + h.aiEmitter.emit("tool-call-end", { + type: "tool-call-end", + workspaceId: h.workspaceId, + messageId: "assistant-1", + toolCallId: "held-tool", + toolName: "bash", + result: {}, + providerExecuted, + timestamp: Date.now(), + }); + expect(h.stopStream).not.toHaveBeenCalled(); + await h.consume(offset); + expect(h.requests[0].hasQueuedMessages?.("tool-end")).toBe(false); + } + await h.complete(); + await h.internal.pendingBashMonitorWakeIdleWaitsByOwner.get(h.workspaceId); + await h.reconciler.reconcile(h.workspaceId); + expect(h.requests).toHaveLength(1); + const history = await h.historyService.getHistoryFromLatestBoundary(h.workspaceId); + expect(history.success && history.data.map((row) => row.role)).toEqual([ + "user", + "assistant", + ]); + expect((await h.reconciler.snapshot(h.workspaceId)).pendingWakeKinds.size).toBe(0); + } finally { + await h.finish(); + } + } + ); + + test("owed attention does not hold a delegated completion open or inherit its closed correlation", async () => { + const h = await createActiveWakeHarness(); + const correlation = { + type: "workspace-turn-task" as const, + taskHandleId: "wst_turn", + ownerWorkspaceId: "parent", + turnId: "turn", + }; + try { + await h.session.sendMessage( + "delegated", + { model: h.model, agentId: "exec", muxMetadata: correlation }, + { synthetic: true, agentInitiated: true } + ); + await h.addAttention(10); + expect(h.service.hasPendingWorkspaceTurnContinuation(h.workspaceId, correlation)).toBe(false); + expect(h.service.hasPendingBashMonitorWakeContinuation(h.workspaceId)).toBe(false); + const next = new Promise((resolve) => h.launched.once("start", resolve)); + await h.complete(); + await next; + expect(h.requests).toHaveLength(2); + expect(h.requests[0].muxMetadata).toEqual(correlation); + expect(h.requests[1].muxMetadata).toBeUndefined(); + } finally { + await h.finish(); + } + }); + + test("full context discard retires owed attention before the active turn becomes idle", async () => { + const h = await createActiveWakeHarness(); + try { + await h.session.sendMessage("original", { model: h.model, agentId: "exec" }); + await h.addAttention(10); + const token = await h.reconciler.beginFullHistoryClear(h.workspaceId); + await h.reconciler.finishFullHistoryClear(token); + await h.complete(); + await h.internal.pendingBashMonitorWakeIdleWaitsByOwner.get(h.workspaceId); + await h.reconciler.reconcile(h.workspaceId); + expect(h.requests).toHaveLength(1); + expect((await h.reconciler.snapshot(h.workspaceId)).pendingWakeKinds.size).toBe(0); + } finally { + await h.finish(); + } + }); + + test("hard Stop retires owed attention without disarming future idle wakes", async () => { + const h = await createActiveWakeHarness(); + try { + await h.session.sendMessage("original", { model: h.model, agentId: "exec" }); + await h.addAttention(10); + spyOn(h.aiService, "stopStream").mockImplementation(async () => { + h.abort("user"); + await h.session.waitForIdle(); + return Ok(undefined); + }); + spyOn(h.aiService, "isStreaming").mockReturnValue(false); + expect( + (await h.service.interruptStream(h.workspaceId, { retireBashMonitorAttention: true })) + .success + ).toBe(true); + await h.internal.pendingBashMonitorWakeIdleWaitsByOwner.get(h.workspaceId); + await h.reconciler.reconcile(h.workspaceId); + expect(h.requests).toHaveLength(1); + expect((await h.reconciler.snapshot(h.workspaceId)).pendingWakeKinds.size).toBe(0); + await h.addAttention(20); + expect(h.requests).toHaveLength(2); + } finally { + await h.finish(); + } + }); + + test("hard Stop does not wait behind a wake admission holding the history lock", async () => { + const h = await createActiveWakeHarness(); + try { + await h.session.sendMessage("original", { model: h.model, agentId: "exec" }); + await h.addAttention(10); + const release = createDeferred(); + const locks = ( + h.service as unknown as { + bashMonitorHistoryLocks: { withLock(key: string, op: () => Promise): Promise }; + } + ).bashMonitorHistoryLocks; + const held = locks.withLock(h.workspaceId, () => release.promise); + spyOn(h.aiService, "stopStream").mockImplementation(async () => { + h.abort("user"); + await h.session.waitForIdle(); + return Ok(undefined); + }); + spyOn(h.aiService, "isStreaming").mockReturnValue(false); + expect( + (await h.service.interruptStream(h.workspaceId, { retireBashMonitorAttention: true })) + .success + ).toBe(true); + release.resolve(); + await held; + await h.internal.pendingBashMonitorWakeIdleWaitsByOwner.get(h.workspaceId); + await h.reconciler.reconcile(h.workspaceId); + expect(h.requests).toHaveLength(1); + } finally { + await h.finish(); + } + }); + + test("a hard Stop whose retirement failed reports it and the retirement lands before any later wake", async () => { + const h = await createActiveWakeHarness(); + try { + await h.session.sendMessage("original", { model: h.model, agentId: "exec" }); + await h.addAttention(10); + const reconcilerInternal = h.reconciler as unknown as { + args: { registry: BashMonitorWakeReconcilerRegistry }; + }; + // Lazy rejection: the stop's I/O crosses a macrotask boundary before the retirement reads + // the registry, and an eager mockRejectedValueOnce promise trips bun's unhandled-rejection + // detector in that gap. + spyOn(reconcilerInternal.args.registry, "listAll").mockImplementationOnce(() => + Promise.reject(new Error("transient registry read")) + ); + spyOn(h.aiService, "stopStream").mockImplementation(async () => { + h.abort("user"); + await h.session.waitForIdle(); + return Ok(undefined); + }); + spyOn(h.aiService, "isStreaming").mockReturnValue(false); + // The stream stopped, but the dismissal is only in memory, so the Stop reports it. + expect( + await h.service.interruptStream(h.workspaceId, { retireBashMonitorAttention: true }) + ).toEqual(Err(STOP_UNRECORDED_MESSAGE)); + expect(h.stopStream).toHaveBeenCalledTimes(1); + await h.internal.pendingBashMonitorWakeIdleWaitsByOwner.get(h.workspaceId); + await h.reconciler.reconcile(h.workspaceId); + // The stop's idle reconcile retried the retirement instead of re-dispatching the output. + expect(h.requests).toHaveLength(1); + expect((await h.reconciler.snapshot(h.workspaceId)).pendingWakeKinds.size).toBe(0); + await h.addAttention(20); + expect(h.requests).toHaveLength(2); + } finally { + await h.finish(); + } + }); + + test("a failed hard Stop keeps owed attention for the idle wake", async () => { + const h = await createActiveWakeHarness(); + try { + await h.session.sendMessage("original", { model: h.model, agentId: "exec" }); + await h.addAttention(10); + h.stopStream.mockResolvedValueOnce(Err("stop failed")); + expect( + (await h.service.interruptStream(h.workspaceId, { retireBashMonitorAttention: true })) + .success + ).toBe(false); + expect((await h.reconciler.snapshot(h.workspaceId)).pendingWakeKinds.size).toBe(2); + await h.complete(); + await h.internal.pendingBashMonitorWakeIdleWaitsByOwner.get(h.workspaceId); + await h.reconciler.reconcile(h.workspaceId); + expect(h.requests).toHaveLength(2); + } finally { + await h.finish(); + } + }); + + test("an interrupt without retireBashMonitorAttention keeps owed attention for the idle wake", async () => { + const h = await createActiveWakeHarness(); + try { + await h.session.sendMessage("original", { model: h.model, agentId: "exec" }); + await h.addAttention(10); + spyOn(h.aiService, "stopStream").mockImplementation(async () => { + h.abort("system"); + await h.session.waitForIdle(); + return Ok(undefined); + }); + spyOn(h.aiService, "isStreaming").mockReturnValue(false); + expect((await h.service.interruptStream(h.workspaceId)).success).toBe(true); + await h.internal.pendingBashMonitorWakeIdleWaitsByOwner.get(h.workspaceId); + await h.reconciler.reconcile(h.workspaceId); + expect(h.requests).toHaveLength(2); + } finally { + await h.finish(); + } + }); + + test("a wake deferred as its idle wait hands off installs the next idle wait", async () => { + const h = await createActiveWakeHarness(); + try { + const internal = h.internal as typeof h.internal & { + scheduleBashMonitorWakeReconcileAfterIdle(ownerWorkspaceId: string): void; + }; + const waits = h.internal.pendingBashMonitorWakeIdleWaitsByOwner; + internal.scheduleBashMonitorWakeReconcileAfterIdle(h.workspaceId); + const handedOff = waits.get(h.workspaceId); + let replacedDuringHandoff: boolean | undefined; + spyOn(h.reconciler, "scheduleReconcile").mockImplementationOnce(() => { + // A turn that started as the wait resolved defers the wake from inside the wait's own + // hand-off; the finished wait must not swallow the re-arm as a duplicate. + internal.scheduleBashMonitorWakeReconcileAfterIdle(h.workspaceId); + replacedDuringHandoff = waits.get(h.workspaceId) !== handedOff; + }); + await handedOff; + expect(replacedDuringHandoff).toBe(true); + await waits.get(h.workspaceId); + } finally { + await h.finish(); + } + }); + + test("hard Stop during a wake's acceptance window keeps the wake from streaming", async () => { + const h = await createActiveWakeHarness(); + try { + let stop: Promise> | undefined; + const unsubscribe = h.session.onChatEvent(({ message: event }) => { + // The wake's user row is emitted past the point of no return and before PREPARING. + if (event.type === "message" && event.role === "user" && stop == null) { + stop = h.service.interruptStream(h.workspaceId, { retireBashMonitorAttention: true }); + } + }); + await h.addAttention(10); + unsubscribe(); + expect(stop).toBeDefined(); + expect((await stop!).success).toBe(true); + expect(h.requests).toHaveLength(0); + expect(h.session.isBusy()).toBe(false); + expect((await h.reconciler.snapshot(h.workspaceId)).pendingWakeKinds.size).toBe(0); + await h.addAttention(20); + expect(h.requests).toHaveLength(1); + } finally { + await h.finish(); + } + }); + + test("a Stop that disables auto-retry interrupts the stream without waiting on the opt-out write", async () => { + const h = await createActiveWakeHarness(); + const release = createDeferred(); + try { + await h.session.sendMessage("original", { model: h.model, agentId: "exec" }); + const stopStream = spyOn(h.aiService, "stopStream").mockImplementation(async () => { + h.abort("user"); + await h.session.waitForIdle(); + return Ok(undefined); + }); + const optOut = h.session.setAutoRetryEnabled.bind(h.session); + spyOn(h.session, "setAutoRetryEnabled").mockImplementation(async (enabled, options) => { + await release.promise; + return optOut(enabled, options); + }); + const stop = h.service.interruptStream(h.workspaceId, { + retireBashMonitorAttention: true, + disableAutoRetry: true, + }); + // The abort reaches the stream while the preference write is still pending. + await waitForCondition(() => stopStream.mock.calls.length === 1); + let stopSettled = false; + void stop.then(() => { + stopSettled = true; + }); + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(stopSettled).toBe(false); + release.resolve(); + expect((await stop).success).toBe(true); + } finally { + release.resolve(); + await h.finish(); + } + }); + + test("a Stop that disables auto-retry withdraws the wake before releasing the retry gate", async () => { + const h = await createActiveWakeHarness(); + try { + let wakeWithdrawnAtOptOut: boolean | undefined; + const optOut = h.session.setAutoRetryEnabled.bind(h.session); + spyOn(h.session, "setAutoRetryEnabled").mockImplementation(async (enabled, options) => { + // The opt-out releases the idle gate a pending wake waits behind; retirement must already + // have withdrawn the wake's dispatch by then. + wakeWithdrawnAtOptOut = h.dispatch.mock.calls[0]?.[0].cancelSignal.aborted; + return optOut(enabled, options); + }); + let stop: Promise> | undefined; + const unsubscribe = h.session.onChatEvent(({ message: event }) => { + if (event.type === "message" && event.role === "user" && stop == null) { + stop = h.service.interruptStream(h.workspaceId, { + retireBashMonitorAttention: true, + disableAutoRetry: true, + }); + } + }); + await h.addAttention(10); + unsubscribe(); + expect((await stop!).success).toBe(true); + expect(wakeWithdrawnAtOptOut).toBe(true); + expect(h.requests).toHaveLength(0); + const sessionInternal = h.session as unknown as { getAutoRetryPreferencePath(): string }; + const persisted = JSON.parse( + await fsPromises.readFile(sessionInternal.getAutoRetryPreferencePath(), "utf-8") + ) as { enabled?: boolean }; + expect(persisted.enabled).toBe(false); + } finally { + await h.finish(); + } + }); + + test("hard Stop during a wake's acceptance window is acknowledged only once the wake's abandon marker is durable", async () => { + const h = await createActiveWakeHarness(); + const release = createDeferred(); + try { + const sessionInternal = h.session as unknown as { + persistAutoRetryState(): Promise; + getAutoRetryPreferencePath(): string; + }; + const persist = sessionInternal.persistAutoRetryState.bind(h.session); + const persisting = createDeferred(); + spyOn(sessionInternal, "persistAutoRetryState").mockImplementation(async () => { + persisting.resolve(); + await release.promise; + return persist(); + }); + let stop: Promise> | undefined; + const unsubscribe = h.session.onChatEvent(({ message: event }) => { + if (event.type === "message" && event.role === "user" && stop == null) { + stop = h.service.interruptStream(h.workspaceId, { retireBashMonitorAttention: true }); + } + }); + const attention = h.addAttention(10); + await persisting.promise; + unsubscribe(); + let stopSettled = false; + void stop!.then(() => { + stopSettled = true; + }); + // Retirement has consumed the signals; Stop still waits for the withdrawn send's marker. + expect((await h.reconciler.snapshot(h.workspaceId)).pendingWakeKinds.size).toBe(0); + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(stopSettled).toBe(false); + release.resolve(); + expect((await stop!).success).toBe(true); + await attention; + const persisted = JSON.parse( + await fsPromises.readFile(sessionInternal.getAutoRetryPreferencePath(), "utf-8") + ) as { startupAutoRetryAbandon?: { reason: string; userMessageId?: string } }; + expect(persisted.startupAutoRetryAbandon?.reason).toBe("aborted"); + expect(persisted.startupAutoRetryAbandon?.userMessageId).toBeDefined(); + expect(h.requests).toHaveLength(0); + } finally { + release.resolve(); + await h.finish(); + } + }); + + test("hard Stop during a wake's acceptance window fails until the withdrawn wake's abandon marker is written", async () => { + const h = await createActiveWakeHarness(); + try { + const sessionInternal = h.session as unknown as { getAutoRetryPreferencePath(): string }; + const preferencePath = sessionInternal.getAutoRetryPreferencePath(); + // A directory at the preference path makes the marker write fail (EISDIR). + await fsPromises.mkdir(preferencePath, { recursive: true }); + let stop: Promise> | undefined; + const unsubscribe = h.session.onChatEvent(({ message: event }) => { + if (event.type === "message" && event.role === "user" && stop == null) { + stop = h.service.interruptStream(h.workspaceId, { retireBashMonitorAttention: true }); + } + }); + await h.addAttention(10); + unsubscribe(); + expect(stop).toBeDefined(); + expect(await stop!).toEqual(Err(STOP_UNRECORDED_MESSAGE)); + expect(h.requests).toHaveLength(0); + expect(h.session.isBusy()).toBe(false); + // The obligation outlives the joined send: a later Stop retries the write once it can succeed. + await fsPromises.rmdir(preferencePath); + expect( + (await h.service.interruptStream(h.workspaceId, { retireBashMonitorAttention: true })) + .success + ).toBe(true); + const persisted = JSON.parse(await fsPromises.readFile(preferencePath, "utf-8")) as { + startupAutoRetryAbandon?: { reason: string; userMessageId?: string }; + }; + expect(persisted.startupAutoRetryAbandon?.reason).toBe("aborted"); + expect(persisted.startupAutoRetryAbandon?.userMessageId).toBeDefined(); + } finally { + await h.finish(); + } + }); + + test("output arriving while a hard Stop waits behind a wake's acceptance stays owed", async () => { + const h = await createActiveWakeHarness(); + const release = createDeferred(); + try { + const acknowledging = createDeferred(); + const reconcilerInternal = h.reconciler as unknown as { + args: { processManager: BashMonitorWakeReconcilerProcessManager }; + }; + spyOn(reconcilerInternal.args.processManager, "acknowledgeMonitorWake").mockImplementation( + async () => { + acknowledging.resolve(); + await release.promise; + } + ); + const attention = h.addAttention(10); + await acknowledging.promise; + // Acceptance holds the reconciler lock; the Stop snapshots the frontier (10) on entry and waits. + const stop = h.service.interruptStream(h.workspaceId, { retireBashMonitorAttention: true }); + const later = h.addAttention(20); + release.resolve(); + expect((await stop).success).toBe(true); + await Promise.all([attention, later]); + // Only the frontier the Stop saw was retired; the newer output woke the idle agent. + expect(h.requests).toHaveLength(1); + } finally { + release.resolve(); + await h.finish(); + } + }); + + test.each([ + ["the failing goal sync", "goal-sync"], + ["the acceptance I/O owed after a failed goal sync", "acceptance"], + ] as const)( + "hard Stop during %s records the abandon marker for the withdrawn wake", + async (_, at) => { + let stop: Promise> | undefined; + const requestStop = () => { + stop ??= h.service.interruptStream(h.workspaceId, { retireBashMonitorAttention: true }); + }; + const h = await createActiveWakeHarness({ + workspaceGoalService: { + assertPricedModelForBudgetedGoal: () => Promise.resolve(Ok(undefined)), + recordStreamStarted: () => undefined, + // Goal sync runs past the point of no return and fails; the failure path still awaits + // acceptance, so Stop can land during either await. + syncGoalModeWithChatTail: () => { + if (at === "goal-sync") requestStop(); + return Promise.reject(new Error("goal sync failed")); + }, + } as unknown as WorkspaceGoalService, + }); + if (at === "acceptance") { + spyOn(h.backgroundProcessManager, "acknowledgeMonitorWake").mockImplementation(requestStop); + } + try { + await h.addAttention(10); + expect(stop).toBeDefined(); + expect((await stop!).success).toBe(true); + const sessionInternal = h.session as unknown as { getAutoRetryPreferencePath(): string }; + const persisted = JSON.parse( + await fsPromises.readFile(sessionInternal.getAutoRetryPreferencePath(), "utf-8") + ) as { startupAutoRetryAbandon?: { reason: string; userMessageId?: string } }; + const history = await h.historyService.getHistoryFromLatestBoundary(h.workspaceId); + const wakeRow = history.success + ? history.data.filter((row) => row.role === "user").at(-1) + : undefined; + expect(wakeRow).toBeDefined(); + expect(persisted.startupAutoRetryAbandon).toEqual({ + reason: "aborted", + userMessageId: wakeRow!.id, + }); + expect((await h.reconciler.snapshot(h.workspaceId)).pendingWakeKinds.size).toBe(0); + expect(h.requests).toHaveLength(0); + } finally { + await h.finish(); + } + } + ); + + test.each(["options", "settings"] as const)( + "wake yields when a turn starts during %s admission", + async (gate) => { + const h = await createActiveWakeHarness(); + const entered = createDeferred(); + const release = createDeferred(); + const options = { model: h.model, agentId: "exec" }; + if (gate === "options") { + spyOn(h.internal, "getDelegatedTurnContinuationSendOptions").mockImplementationOnce( + async () => { + entered.resolve(); + await release.promise; + return options; + } + ); + } else { + const internal = h.service as unknown as { + maybePersistAISettingsFromOptions(): Promise; + }; + spyOn(internal, "maybePersistAISettingsFromOptions").mockImplementationOnce(async () => { + entered.resolve(); + await release.promise; + }); + } + try { + const attention = h.addAttention(10); + await entered.promise; + await h.session.sendMessage("original", options); + release.resolve(); + await attention; + expect(h.requests).toHaveLength(1); + expect(h.requests[0].hasQueuedMessages?.("tool-end")).toBe(false); + await h.consume(10); + await h.complete(); + await h.reconciler.reconcile(h.workspaceId); + expect(h.requests).toHaveLength(1); + } finally { + release.resolve(); + await h.finish(); + } + } + ); + + test("unconsumed attention coalesces after natural completion and idle attention starts promptly", async () => { + const h = await createActiveWakeHarness(); + try { + await h.session.sendMessage("original", { model: h.model, agentId: "exec" }); + await h.addAttention(10); + await h.reconciler.reconcile(h.workspaceId); + expect(h.requests).toHaveLength(1); + const next = new Promise((resolve) => h.launched.once("start", resolve)); + await h.complete(); + await next; + expect(h.requests).toHaveLength(2); + await h.reconciler.reconcile(h.workspaceId); + expect((await h.reconciler.snapshot(h.workspaceId)).pendingWakeKinds.size).toBe(0); + await h.complete(); + await h.addAttention(20); + expect(h.requests).toHaveLength(3); + } finally { + await h.finish(); + } + }); + + test.each([false, true])( + "manual tool-end input takes precedence over owed bash attention (native=%s)", + async (providerExecuted) => { + const h = await createActiveWakeHarness(); + try { + await h.session.sendMessage("original", { model: h.model, agentId: "exec" }); + await h.addAttention(10); + expect( + ( + await h.service.sendMessage(h.workspaceId, "manual", { + model: h.model, + agentId: "exec", + queueDispatchMode: "tool-end", + }) + ).success + ).toBe(true); + expect(h.requests[0].hasQueuedMessages?.("tool-end")).toBe(true); + h.aiEmitter.emit("tool-call-end", { + type: "tool-call-end", + workspaceId: h.workspaceId, + messageId: "assistant-1", + toolCallId: "tool", + toolName: "bash", + result: {}, + providerExecuted, + timestamp: Date.now(), + }); + expect(h.stopStream).toHaveBeenCalledTimes(providerExecuted ? 1 : 0); + await h.consume(10); + const next = new Promise((resolve) => h.launched.once("start", resolve)); + if (providerExecuted) { + h.abort("system"); + } else { + await h.complete("tool-calls"); + } + await next; + expect(h.requests).toHaveLength(2); + const history = await h.historyService.getHistoryFromLatestBoundary(h.workspaceId); + expect( + history.success && + history.data.filter((row) => row.role === "user").map((row) => row.parts[0]) + ).toEqual([ + expect.objectContaining({ type: "text", text: "original" }), + expect.objectContaining({ type: "text", text: "manual" }), + ]); + } finally { + await h.finish(); + } + } + ); + test("monitor lifecycle and shown-output events poke the reconciler", async () => { const { service, events, cleanup } = await createWakeWiringService(); const scheduleReconcile = mock(() => undefined); @@ -751,7 +1774,6 @@ describe("WorkspaceService bash monitor wake reconciler wiring", () => { ownerWorkspaceId: string; prompt: string; muxMetadata: { type: "bash-monitor-wake"; records: [] }; - dedupeKey: string; cancelSignal: AbortSignal; onAccepted(): Promise; onDeferred(): Promise; @@ -764,7 +1786,6 @@ describe("WorkspaceService bash monitor wake reconciler wiring", () => { ownerWorkspaceId: workspaceId, prompt: "wake", muxMetadata: { type: "bash-monitor-wake", records: [] }, - dedupeKey: "wake", cancelSignal: new AbortController().signal, onAccepted, onDeferred, @@ -777,7 +1798,7 @@ describe("WorkspaceService bash monitor wake reconciler wiring", () => { } }); - test("active session-backed streams queue monitor wakes at tool end", async () => { + test("active session-backed streams defer monitor attention until idle", async () => { const { config, service, cleanup } = await createWakeWiringService(); const workspaceId = "streaming-wake-owner"; await config.addWorkspace("/tmp/streaming-wake-project", { @@ -787,25 +1808,11 @@ describe("WorkspaceService bash monitor wake reconciler wiring", () => { projectPath: "/tmp/streaming-wake-project", runtimeConfig: { type: "local" }, }); - let queuedMode: string | undefined; - let queuedCancelState: { canceledBeforeAcceptance: boolean } | undefined; - const sendMessage = mock( - ( - _workspaceId: string, - _prompt: string, - options: { queueDispatchMode?: string }, - internal?: { cancelState?: { canceledBeforeAcceptance: boolean } } - ) => { - queuedMode = options.queueDispatchMode; - queuedCancelState = internal?.cancelState; - return Promise.resolve(Ok(undefined)); - } - ); + const sendMessage = mock(() => Promise.resolve(Ok(undefined))); const afterIdle = mock(() => undefined); const internal = service as unknown as { aiService: { isStreaming(workspaceId: string): boolean }; hasPendingQueuedOrPreparingTurn(workspaceId: string): boolean; - isBusyForMessage(workspaceId: string): boolean; scheduleBashMonitorWakeReconcileAfterIdle(workspaceId: string): void; getDelegatedTurnContinuationSendOptions(workspaceId: string): Promise; sendMessage: typeof sendMessage; @@ -813,16 +1820,15 @@ describe("WorkspaceService bash monitor wake reconciler wiring", () => { ownerWorkspaceId: string; prompt: string; muxMetadata: { type: "bash-monitor-wake"; records: [] }; - dedupeKey: string; cancelSignal: AbortSignal; onAccepted(): Promise; onDeferred(): Promise; }): Promise<"in-flight" | "deferred">; }; try { + spyOn(service.getOrCreateSession(workspaceId), "isBusy").mockReturnValue(true); internal.aiService = { isStreaming: () => true }; internal.hasPendingQueuedOrPreparingTurn = () => false; - internal.isBusyForMessage = () => true; internal.scheduleBashMonitorWakeReconcileAfterIdle = afterIdle; internal.getDelegatedTurnContinuationSendOptions = () => Promise.resolve({}); internal.sendMessage = sendMessage; @@ -831,122 +1837,113 @@ describe("WorkspaceService bash monitor wake reconciler wiring", () => { ownerWorkspaceId: workspaceId, prompt: "wake", muxMetadata: { type: "bash-monitor-wake", records: [] }, - dedupeKey: "wake", cancelSignal: new AbortController().signal, onAccepted: () => Promise.resolve(), onDeferred: () => Promise.resolve(), }); - expect(outcome).toBe("in-flight"); - expect(sendMessage).toHaveBeenCalledTimes(1); - expect(queuedMode).toBe("tool-end"); - expect(queuedCancelState).toEqual({ canceledBeforeAcceptance: false }); - expect(afterIdle).not.toHaveBeenCalled(); + expect(outcome).toBe("deferred"); + expect(sendMessage).not.toHaveBeenCalled(); + expect(afterIdle).toHaveBeenCalledWith(workspaceId); } finally { await cleanup(); } }); - test("withdrawing a queued monitor wake removes it and releases its dedupe key", async () => { + test("pending mid-stream compaction defers monitor attention like an active turn", async () => { const { config, service, cleanup } = await createWakeWiringService(); - const workspaceId = "withdrawn-wake-owner"; - await config.addWorkspace("/tmp/withdrawn-wake-project", { + const workspaceId = "compacting-wake-owner"; + await config.addWorkspace("/tmp/compacting-wake-project", { id: workspaceId, name: workspaceId, - projectName: "withdrawn-wake-project", - projectPath: "/tmp/withdrawn-wake-project", + projectName: "compacting-wake-project", + projectPath: "/tmp/compacting-wake-project", runtimeConfig: { type: "local" }, }); - const session = service.getOrCreateSession(workspaceId); - const queuedModes: Array<"tool-end" | "turn-end" | null> = []; - // The real sendMessage queues behind a busy session; mirror only that branch. - const sendMessage = mock( - ( - _workspaceId: string, - prompt: string, - options: SendMessageOptions, - internal?: { - synthetic?: boolean; - agentInitiated?: boolean; - queueDedupeKey?: string; - removableQueueDedupeKey?: boolean; - cancelState?: { canceledBeforeAcceptance: boolean }; - cancelSignal?: AbortSignal; - onCanceled?: (reason: string) => Promise | void; - } - ) => { - queuedModes.push( - session.queueMessage(prompt, options, { - synthetic: internal?.synthetic, - agentInitiated: internal?.agentInitiated, - dedupeKey: internal?.queueDedupeKey, - removableDedupeKey: internal?.removableQueueDedupeKey, - cancelState: internal?.cancelState, - cancelSignal: internal?.cancelSignal, - onCanceled: internal?.onCanceled, - }) - ); - return Promise.resolve(Ok(undefined)); - } - ); + const sendMessage = mock(() => Promise.resolve(Ok(undefined))); + const afterIdle = mock(() => undefined); const internal = service as unknown as { - aiService: { isStreaming(workspaceId: string): boolean }; - hasPendingQueuedOrPreparingTurn(workspaceId: string): boolean; - isBusyForMessage(workspaceId: string): boolean; + scheduleBashMonitorWakeReconcileAfterIdle(workspaceId: string): void; getDelegatedTurnContinuationSendOptions(workspaceId: string): Promise; sendMessage: typeof sendMessage; dispatchBashMonitorWake(dispatch: { ownerWorkspaceId: string; prompt: string; muxMetadata: { type: "bash-monitor-wake"; records: [] }; - dedupeKey: string; cancelSignal: AbortSignal; onAccepted(): Promise; onDeferred(): Promise; }): Promise<"in-flight" | "deferred">; }; - const dedupeKey = "bash-monitor-wake:" + workspaceId + ":dispatch-1"; - const onDeferred = mock(() => Promise.resolve()); - const dispatch = (cancelSignal: AbortSignal) => - internal.dispatchBashMonitorWake({ + try { + // Between the stopped stream and its compaction request the coordinator is idle and no + // stream is running; only the session's pending flag marks the turn work. + const session = service.getOrCreateSession(workspaceId); + Reflect.set(session, "midStreamCompactionPending", true); + internal.scheduleBashMonitorWakeReconcileAfterIdle = afterIdle; + internal.getDelegatedTurnContinuationSendOptions = () => Promise.resolve({}); + internal.sendMessage = sendMessage; + + const outcome = await internal.dispatchBashMonitorWake({ ownerWorkspaceId: workspaceId, prompt: "wake", muxMetadata: { type: "bash-monitor-wake", records: [] }, - dedupeKey, - cancelSignal, + cancelSignal: new AbortController().signal, onAccepted: () => Promise.resolve(), - onDeferred, + onDeferred: () => Promise.resolve(), }); - try { - internal.aiService = { isStreaming: () => true }; - internal.hasPendingQueuedOrPreparingTurn = () => false; - internal.isBusyForMessage = () => true; - internal.getDelegatedTurnContinuationSendOptions = () => Promise.resolve({}); - internal.sendMessage = sendMessage; - - const controller = new AbortController(); - expect(await dispatch(controller.signal)).toBe("in-flight"); - expect(session.hasQueuedMessages("tool-end")).toBe(true); - controller.abort("output already shown"); - expect(session.hasQueuedMessages()).toBe(false); - expect(service.removeQueuedMessagesByDedupeKeyPrefix(workspaceId, dedupeKey)).toEqual(Ok(0)); - await new Promise((resolve) => setTimeout(resolve, 0)); - expect(onDeferred).toHaveBeenCalledTimes(1); - - // Already withdrawn at dispatch: never reaches the send, so nothing can be enqueued. - expect(await dispatch(controller.signal)).toBe("deferred"); - expect(sendMessage).toHaveBeenCalledTimes(1); - expect(session.hasQueuedMessages()).toBe(false); - expect(onDeferred).toHaveBeenCalledTimes(1); - - expect(await dispatch(new AbortController().signal)).toBe("in-flight"); - expect(queuedModes).toEqual(["tool-end", "tool-end"]); + expect(outcome).toBe("deferred"); + expect(sendMessage).not.toHaveBeenCalled(); + expect(afterIdle).toHaveBeenCalledWith(workspaceId); } finally { await cleanup(); } }); + test("withdrawn idle wake rolls back admission and permits a fresh delivery", async () => { + const h = await createActiveWakeHarness(); + const entered = createDeferred(); + const release = createDeferred(); + const append = h.historyService.appendToHistory.bind(h.historyService); + spyOn(h.historyService, "appendToHistory").mockImplementationOnce(async (...args) => { + entered.resolve(); + await release.promise; + return append(...args); + }); + const controller = new AbortController(); + const accepted = mock(() => Promise.resolve()); + const deferred = mock(() => Promise.resolve()); + const send = (cancelSignal: AbortSignal) => + h.internal.dispatchBashMonitorWake({ + ownerWorkspaceId: h.workspaceId, + prompt: "wake", + muxMetadata: { type: "bash-monitor-wake", records: [] }, + cancelSignal, + onAccepted: accepted, + onDeferred: deferred, + }); + try { + const dispatch = send(controller.signal); + await entered.promise; + controller.abort(); + release.resolve(); + await dispatch; + expect(accepted).not.toHaveBeenCalled(); + expect(deferred).toHaveBeenCalledTimes(1); + expect(h.requests).toHaveLength(0); + expect(h.session.hasQueuedMessages()).toBe(false); + const history = await h.historyService.getHistoryFromLatestBoundary(h.workspaceId); + expect(history.success && history.data).toEqual([]); + await send(new AbortController().signal); + expect(h.requests).toHaveLength(1); + expect(accepted).toHaveBeenCalledTimes(1); + } finally { + release.resolve(); + await h.finish(); + } + }); + test("superseding queued sub-agent progress skips its continuation-failure callbacks", async () => { const { config, service, cleanup } = await createWakeWiringService(); const workspaceId = "superseded-progress-owner"; @@ -6458,6 +7455,7 @@ describe("WorkspaceService truncateHistory goal acknowledgment", () => { const session = { closingSignal: new AbortController().signal, isBusy: mock(() => busy), + hasActiveOrPendingTurnWork: mock(() => busy), hasQueuedMessages: mock(() => false), hasPendingAutoRetry: mock(() => pendingAutoRetry), waitForIdle, @@ -6500,6 +7498,29 @@ describe("WorkspaceService truncateHistory goal acknowledgment", () => { } }); + test("idle wait outlasts a pending mid-stream compaction request", async () => { + const { workspaceService, cleanup } = await createServices(); + const workspaceId = "idle-wait-pending-compaction"; + const session = workspaceService.getOrCreateSession(workspaceId); + const settle = Reflect.get(session, "settleMidStreamCompaction") as () => void; + try { + Reflect.set(session, "midStreamCompactionPending", true); + let resolved = false; + const waitPromise = workspaceService.waitForIdleAndNoQueuedMessages(workspaceId).then(() => { + resolved = true; + }); + await drainPendingDispatches(); + expect(resolved).toBe(false); + + // The compaction request never became a turn: no stream event fires, only the window closes. + settle.call(session); + await waitPromise; + expect(resolved).toBe(true); + } finally { + await cleanup(); + } + }); + test("destructive clear waits for startup monitor recovery discovery", async () => { const { historyService, workspaceService, cleanup } = await createServices(); const workspaceId = "clear-waits-for-monitor-recovery"; diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index 2c0dd025e21..d1afc4e2773 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -29,6 +29,7 @@ import { reassignPinnedTimestamps, } from "@/common/utils/pin"; import { SCRATCH_PROJECT_CONFIG_KEY } from "@/common/constants/scratch"; +import { STOP_UNRECORDED_MESSAGE } from "@/common/constants/workspace"; import { MULTI_PROJECT_CONFIG_KEY } from "@/common/constants/multiProject"; import type { CompactionCompletionMetadata } from "@/common/types/compaction"; import { ProvidersConfigStore, SecretsStore, type Config } from "@/node/config"; @@ -117,6 +118,7 @@ import { sliceMessagesForProviderFromLatestContextBoundary, } from "@/common/utils/messages/compactionBoundary"; import { isNonNegativeInteger, isPositiveInteger } from "@/common/utils/numbers"; +import { isPlainObject } from "@/common/utils/isPlainObject"; import { deriveTodoStatus } from "@/common/utils/todoList"; import { createContextResetBoundaryMessageId } from "@/node/services/utils/messageIds"; import { fileExists } from "@/node/utils/runtime/fileExists"; @@ -318,6 +320,7 @@ import { type BashMonitorWakeDispatch, type BashMonitorWakeDispatchOutcome, type BashMonitorWakeReconcilerSnapshot, + type DeliveredWakeRecord, } from "@/node/services/bashMonitorWakeReconciler"; import type { WorkspaceLifecycleHooks } from "@/node/services/workspaceLifecycleHooks"; import { @@ -1855,6 +1858,8 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { private readonly bashMonitorWakeReconciler: BashMonitorWakeReconciler; private readonly constructedAtMs = Date.now(); private readonly pendingBashMonitorWakeIdleWaitsByOwner = new Map>(); + /** The wake send in flight per owner (at most one: dispatch runs under the history lock). */ + private readonly inFlightBashMonitorWakeSendsByOwner = new Map>(); private readonly bashMonitorHistoryLocks = new MutexMap(); private readonly bashMonitorRecoveryPromise: Promise; private readonly pendingBashMonitorPersistenceByWorkspace = new Map>>(); @@ -2377,7 +2382,7 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { pullMonitorWakeSignals: (ownerWorkspaceId) => typeof monitorManager.pullMonitorWakeSignals === "function" ? monitorManager.pullMonitorWakeSignals(ownerWorkspaceId) - : Promise.resolve([]), + : [], getMonitorWakeDeliveryState: (processId, originNotAfterMs) => typeof monitorManager.getMonitorWakeDeliveryState === "function" ? monitorManager.getMonitorWakeDeliveryState(processId, originNotAfterMs) @@ -2397,6 +2402,8 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { : undefined, }, registry: this.bashMonitorRegistryStore, + deliveredWakes: (ownerWorkspaceId, sinceMs) => + this.listDeliveredBashMonitorWakes(ownerWorkspaceId, sinceMs), onWake: (dispatch) => this.dispatchBashMonitorWake(dispatch), }); if (typeof this.backgroundProcessManager.on === "function") { @@ -2546,15 +2553,69 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { error, }); }) - .then(() => this.scheduleBashMonitorWakeReconcile(ownerWorkspaceId)) - .finally(() => { + .then(() => { + // Release the slot before scheduling: the reconcile may find a new turn already running + // and must be able to install the next idle wait. if (this.pendingBashMonitorWakeIdleWaitsByOwner.get(ownerWorkspaceId) === promise) { this.pendingBashMonitorWakeIdleWaitsByOwner.delete(ownerWorkspaceId); } + this.scheduleBashMonitorWakeReconcile(ownerWorkspaceId); }); this.pendingBashMonitorWakeIdleWaitsByOwner.set(ownerWorkspaceId, promise); } + /** + * Wake records in transcript rows written since `sinceMs`, scanning newest-first across the + * compaction archive too because a delivered row stays proof of delivery after it leaves the + * window the model sees. A failed read rejects so the reconciler holds dispatch in its retry + * backoff instead of risking a duplicate row. + */ + private async listDeliveredBashMonitorWakes( + ownerWorkspaceId: string, + sinceMs: number + ): Promise { + const records: DeliveredWakeRecord[] = []; + const result = await this.historyService.iterateFullHistory( + ownerWorkspaceId, + "backward", + (messages) => { + // A wake row is appended after its process was created, so once a whole chunk predates + // every process being checked the rest of history cannot carry one. + let predatesAll = messages.length > 0; + for (const message of messages) { + const muxMetadata = message.metadata?.muxMetadata; + // A wake that triggered on-send compaction persists only the compaction request, with + // the wake's metadata nested as its follow-up. + const wake = + muxMetadata?.type === "bash-monitor-wake" + ? muxMetadata + : getCompactionFollowUpContent(muxMetadata)?.muxMetadata; + if (message.role === "user" && wake?.type === "bash-monitor-wake") { + // Persisted metadata is unvalidated: a malformed row must degrade to "not delivered" + // rather than fail the scan, which would hold every wake of this owner. + const rows: unknown = wake.records; + if (Array.isArray(rows)) { + for (const row of rows) { + if ( + isPlainObject(row) && + typeof row.processId === "string" && + typeof row.wakeUpdatedAt === "string" + ) { + records.push({ processId: row.processId, wakeUpdatedAt: row.wakeUpdatedAt }); + } + } + } + } + const timestamp = message.metadata?.timestamp; + if (!(typeof timestamp === "number" && timestamp < sinceMs)) predatesAll = false; + } + return !predatesAll; + } + ); + if (!result.success) throw new Error(result.error); + return records; + } + private async dispatchBashMonitorWake( dispatch: BashMonitorWakeDispatch ): Promise { @@ -2566,14 +2627,27 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { this.notifyBashMonitorWakeStateChanged(ownerWorkspaceId); return "in-flight"; } + // sendMessage refuses archived workspaces and no session exists to wait on, so an after-idle + // retry would spin; the wake stays owed and unarchive reconciles it. + if ( + this.archivingWorkspaces.has(ownerWorkspaceId) || + isWorkspaceArchived(entry.workspace.archivedAt, entry.workspace.unarchivedAt) + ) { + return "deferred"; + } const hasPendingTurn = this.hasPendingQueuedOrPreparingTurn(ownerWorkspaceId); - const hasSessionBackedBusyState = this.isBusyForMessage(ownerWorkspaceId); + // Pending mid-stream compaction counts as turn work: the session reads idle between the + // stopped stream and its compaction request, which the session sends directly. + const hasSessionBackedBusyState = + this.sessions.get(ownerWorkspaceId)?.hasActiveOrPendingTurnWork() === true; const hasAiServiceStream = this.aiService.isStreaming(ownerWorkspaceId); - if (hasPendingTurn || (hasSessionBackedBusyState && !hasAiServiceStream)) { + // Cancelable attention must not cut a turn that can consume it in its current tool call. + // Keep it outside the queue so later manual tool-end input cannot be held behind it. + if (hasPendingTurn || hasSessionBackedBusyState) { this.scheduleBashMonitorWakeReconcileAfterIdle(ownerWorkspaceId); return "deferred"; } - if (hasAiServiceStream && !hasSessionBackedBusyState) { + if (hasAiServiceStream) { return "deferred"; } const sendOptions = @@ -2584,43 +2658,24 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { return "deferred"; } - // Withdrawn while awaiting send options above: the abort listener below would never - // fire, and send preflight (which persists AI settings) has nothing left to admit. + // Withdrawal during send-option resolution must not enter preflight or persist settings. if (dispatch.cancelSignal.aborted) return "deferred"; let accepted = false; - // A queued wake can be superseded after dequeue. Share cancellation state so - // AgentSession can release PREPARING when cancellation wins before acceptance. - const cancelState = { canceledBeforeAcceptance: false }; - // Withdrawal (output already shown, process discarded, history cleared) must - // free the queue slot now, not at stream end: a lingering entry keeps the - // workspace reported busy and its dedupe key held. The key is unique per - // dispatch, so this cannot drop a newer wake's entry. - dispatch.cancelSignal.addEventListener( - "abort", - () => { - this.removeQueuedMessagesByDedupeKeyPrefix(ownerWorkspaceId, dispatch.dedupeKey, { - cancelReason: "Bash monitor wake withdrawn before dispatch.", - }); - }, - { once: true } - ); - const sendResult = await this.sendMessage( + const send = this.sendMessage( ownerWorkspaceId, dispatch.prompt, { ...sendOptions, - queueDispatchMode: "tool-end", muxMetadata: dispatch.muxMetadata, }, { skipAutoResumeReset: true, synthetic: true, agentInitiated: true, - cancelState, + requireIdle: true, cancelSignal: dispatch.cancelSignal, - queueDedupeKey: dispatch.dedupeKey, - removableQueueDedupeKey: true, + withdrawAcceptedOnCancel: true, onAccepted: async () => { accepted = true; await dispatch.onAccepted(); @@ -2637,6 +2692,16 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { }, } ); + // Published so a hard Stop that withdraws this wake can join it (see interruptStream). + this.inFlightBashMonitorWakeSendsByOwner.set(ownerWorkspaceId, send); + let sendResult: Awaited; + try { + sendResult = await send; + } finally { + if (this.inFlightBashMonitorWakeSendsByOwner.get(ownerWorkspaceId) === send) { + this.inFlightBashMonitorWakeSendsByOwner.delete(ownerWorkspaceId); + } + } if (!sendResult.success && !accepted) { this.scheduleBashMonitorWakeReconcileAfterIdle(ownerWorkspaceId); return "deferred"; @@ -9014,26 +9079,34 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { } } - // Lifecycle hooks run *after* we persist unarchivedAt. - // - // Why best-effort: Unarchive is a quick UI action and should not fail permanently due to a - // start error (e.g., Coder workspace start). - if (this.workspaceLifecycleHooks && hookMetadata) { - await this.workspaceLifecycleHooks.runAfterUnarchive({ - workspaceId, - workspaceMetadata: hookMetadata, - }); - } + // Restoration succeeded, so the unarchive is final from here: monitor attention held while + // archived (see dispatchBashMonitorWake) wakes after lifecycle startup below, and still + // wakes when a follow-up step throws, since a retried unarchive would take the + // !didUnarchive exit and never reach this point again. + try { + // Lifecycle hooks run *after* we persist unarchivedAt. + // + // Why best-effort: Unarchive is a quick UI action and should not fail permanently due to a + // start error (e.g., Coder workspace start). + if (this.workspaceLifecycleHooks && hookMetadata) { + await this.workspaceLifecycleHooks.runAfterUnarchive({ + workspaceId, + workspaceMetadata: hookMetadata, + }); + } - if (this.workspaceLifecycleHooks || this.worktreeArchiveSnapshotService) { - await this.emitCurrentWorkspaceMetadata(workspaceId); - } + if (this.workspaceLifecycleHooks || this.worktreeArchiveSnapshotService) { + await this.emitCurrentWorkspaceMetadata(workspaceId); + } - await this.syncCodeWorkspaceFiles({ - projectPath, - projects: hookMetadata?.projects, - subProjectPath: hookMetadata?.subProjectPath, - }); + await this.syncCodeWorkspaceFiles({ + projectPath, + projects: hookMetadata?.projects, + subProjectPath: hookMetadata?.subProjectPath, + }); + } finally { + this.scheduleBashMonitorWakeReconcile(workspaceId); + } // Archived owners park workflow terminal wakes unsettled; reconcile so an idle // workspace does not stay silent until the interval sweep. Only AFTER snapshot @@ -10965,6 +11038,7 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { goalId: internal?.goalId, cancelState: internal?.cancelState, cancelSignal: internal?.cancelSignal, + withdrawAcceptedOnCancel: internal?.withdrawAcceptedOnCancel, onCanceled: internal?.onCanceled, onAccepted: internal?.onAccepted, onAcceptedPreStreamFailure: internal?.onAcceptedPreStreamFailure, @@ -11266,6 +11340,7 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { startStreamInBackground: internal?.startStreamInBackground, cancelState: internal?.cancelState, cancelSignal: internal?.cancelSignal, + withdrawAcceptedOnCancel: internal?.withdrawAcceptedOnCancel, // Same authoring-time race as the queued path: the goal-creating // stream can end during the preflight awaits above, making a fresh // goal visible after the user hit enter but before this dispatch. @@ -11651,7 +11726,13 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { async interruptStream( workspaceId: string, - options?: { soft?: boolean; abandonPartial?: boolean; sendQueuedImmediately?: boolean } + options?: { + soft?: boolean; + abandonPartial?: boolean; + sendQueuedImmediately?: boolean; + retireBashMonitorAttention?: boolean; + disableAutoRetry?: boolean; + } ): Promise> { let releaseHardStopLatch: (() => void) | undefined; try { @@ -11671,7 +11752,68 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { } const session = this.getOrCreateSession(workspaceId); - const stopResult = await session.interruptStream(options); + // Only a user Stop dismisses owed attention; internal interrupts (goal promotion, archive, + // ACP disconnect, send-now) must not lose monitor output. Start retiring before the abort: + // consumeCurrent withdraws an in-flight dispatch synchronously and reserves the reconciler + // lock ahead of the reconcile this abort's idle transition triggers, so the abort itself + // never waits behind acceptance I/O. The durable consumption commits only once the stop + // succeeded: a failed stop leaves the agent running, so its output stays owed. Monitors stay + // armed for new output. Retirement I/O that fails keeps the frontier owed in memory (the + // reconciler retries it before any dispatch, as does the next Stop) but fails this Stop + // below: that obligation is not durable, so a restart before the retry could wake the agent + // on the dismissed output. Never behind the history lock (a wake admission holds it across + // stream construction). + const retiring = options?.retireBashMonitorAttention === true; + const withdrawnWakeSend = retiring + ? this.inFlightBashMonitorWakeSendsByOwner.get(workspaceId) + : undefined; + let settleStop!: (stopped: boolean) => void; + const stopSettled = new Promise((resolve) => { + settleStop = resolve; + }); + let retirementRecorded = true; + const retirement = retiring + ? this.bashMonitorWakeReconciler + .consumeCurrent(workspaceId, () => stopSettled) + .catch((error: unknown) => { + retirementRecorded = false; + log.warn("Failed to retire bash monitor attention before Stop", { + workspaceId, + error, + }); + }) + : undefined; + // The opt-out starts only after retirement reserved the reconciler lock above: disabling + // retry releases the idle gate a pending wake may be waiting behind, and that wake must + // find its attention withdrawn, not a window to start a turn after the user's Stop. The + // interrupt does not wait for the opt-out's disk write (a stuck write must not keep the + // stream running); the write is joined and verified below, and a failure fails the Stop. + const disabling = options?.disableAutoRetry === true; + const optOut = disabling + ? session.setAutoRetryEnabled(false).catch((error: unknown) => { + log.warn("Failed to disable auto-retry during Stop", { workspaceId, error }); + }) + : undefined; + let stopResult: Result | undefined; + try { + stopResult = await session.interruptStream(options); + } finally { + settleStop(stopResult?.success === true); + } + await retirement; + await optOut; + // A wake withdrawn past its point of no return (durable row, not yet PREPARING, so the + // session interrupt above saw idle) records the startup abandon marker for that row on every + // exit before it resolves, including a failed goal sync or acceptance (see + // abandonWithdrawnSend in AgentSession.sendMessage). Stop is acknowledged after it settles: a + // forced exit right after Stop must not leave the row eligible for startup replay. The send's + // own result is the dispatch's to report; a marker (or a RetryBarrier Stop's opt-out) still + // unrecorded after the session retried the write fails the Stop below, on this and every later + // Stop, so the obligation is not lost with the joined send. + await withdrawnWakeSend?.catch(() => undefined); + const stopRecorded = + !(retiring || disabling) || + ((await session.recordPendingAutoRetryState()) && retirementRecorded); if (!stopResult.success) { // Interrupt failed, so clear hard-interrupt suppression we set above. if (!options?.soft) { @@ -11721,6 +11863,10 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { session.restoreQueueToInput(); } + if (!stopRecorded) { + log.error("Stop left stopped work eligible to resume on restart", { workspaceId }); + return Err(STOP_UNRECORDED_MESSAGE); + } return Ok(undefined); } catch (error) { if (!options?.soft) { @@ -12045,12 +12191,22 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { } if (session.closingSignal.aborted) throw new Error(WORKSPACE_IDLE_WAIT_CANCELED_MESSAGE); - while (session.isBusy() || session.hasQueuedMessages() || session.hasPendingAutoRetry()) { + // Pending mid-stream compaction is turn work the coordinator cannot see: the session reads + // idle until the compaction request claims PREPARING. + const hasTurnWork = () => + session.hasActiveOrPendingTurnWork() || + session.hasQueuedMessages() || + session.hasPendingAutoRetry(); + while (hasTurnWork()) { if (session.closingSignal.aborted) throw new Error(WORKSPACE_IDLE_WAIT_CANCELED_MESSAGE); if (session.isBusy()) { await session.waitForIdle(); continue; } + if (session.hasActiveOrPendingTurnWork()) { + await session.waitForMidStreamCompactionSettled(); + continue; + } await new Promise((resolve) => { let settled = false; @@ -12071,17 +12227,13 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { eventType === "stream-lifecycle"; const queuedOrRetryCleared = (eventType === "queued-message-changed" || eventType === "auto-retry-abandoned") && - !session.hasQueuedMessages() && - !session.hasPendingAutoRetry(); + !hasTurnWork(); if (retryStartedOrTurnPhaseChanged || queuedOrRetryCleared) { finish(); } }); session.closingSignal.addEventListener("abort", finish, { once: true }); - if ( - session.closingSignal.aborted || - (!session.hasQueuedMessages() && !session.hasPendingAutoRetry()) - ) { + if (session.closingSignal.aborted || !hasTurnWork()) { finish(); } }); @@ -14704,11 +14856,10 @@ export class WorkspaceService extends EventEmitter implements WorkspaceHost { } /** - * Send options for continuing a STILL-OPEN delegated workspace turn (bash-monitor - * wakes cut turns at tool boundaries). The delegated prompt's persisted - * retrySendOptions carry the turn's own settings — including per-turn overrides - * (agentId, model, strictAgentResolution) that are deliberately NOT in the - * workspace's persisted defaults when the launch used skipAiSettingsPersistence — + * Send options for continuing a STILL-OPEN delegated workspace turn. The delegated + * prompt's persisted retrySendOptions carry the turn's own settings — including + * per-turn overrides (agentId, model, strictAgentResolution) that are deliberately NOT + * in the workspace's persisted defaults when the launch used skipAiSettingsPersistence — * so resolving from workspace defaults would continue the turn under the wrong * agent. Openness is decided by the same rule as workspace-turn correlation * (inheritOpenWorkspaceTurnMetadata): only a correlated assistant cut with diff --git a/tests/ipc/acp.promptCorrelation.test.ts b/tests/ipc/acp.promptCorrelation.test.ts index 9109ade9411..9f34d791789 100644 --- a/tests/ipc/acp.promptCorrelation.test.ts +++ b/tests/ipc/acp.promptCorrelation.test.ts @@ -1,4 +1,5 @@ import { AgentSideConnection, PROTOCOL_VERSION, ndJsonStream } from "@agentclientprotocol/sdk"; +import { STOP_UNRECORDED_MESSAGE } from "../../src/common/constants/workspace"; import type { OnChatMode, WorkspaceChatMessage } from "../../src/common/orpc/types"; import { MuxAgent } from "../../src/node/acp/agent"; import type { ORPCClient, ServerConnection } from "../../src/node/acp/serverConnection"; @@ -885,6 +886,7 @@ describe("ACP prompt stream correlation", () => { expect(harness.interruptCalls).toEqual([ { workspaceId: newSessionResponse.sessionId, + options: { retireBashMonitorAttention: true }, }, ]); @@ -892,6 +894,25 @@ describe("ACP prompt stream correlation", () => { await harness.connectionClosed; }); + it("settles pending prompts as cancelled when the Stop stopped the stream but was not recorded", async () => { + const harness = createHarness({ + interruptStream: async () => ({ success: false, error: STOP_UNRECORDED_MESSAGE }), + }); + const { newSessionResponse, promptPromise } = await createDefaultPromptTurn(harness); + + await expect(harness.agent.cancel({ sessionId: newSessionResponse.sessionId })).rejects.toThrow( + STOP_UNRECORDED_MESSAGE + ); + + await expect(promptPromise).resolves.toEqual({ + stopReason: "cancelled", + usage: undefined, + }); + + harness.closeConnection(); + await harness.connectionClosed; + }); + it("accepts correlated terminal events even when messageId is empty", async () => { const harness = createHarness(); const { newSessionResponse, promptPromise, promptCorrelationId } =