diff options
Diffstat (limited to 'src/features')
20 files changed, 1030 insertions, 48 deletions
diff --git a/src/features/chat/index.ts b/src/features/chat/index.ts index cf57cea..2f98a0e 100644 --- a/src/features/chat/index.ts +++ b/src/features/chat/index.ts @@ -12,13 +12,23 @@ export type { EffortOption, ReasoningEffortSaveResult, SaveReasoningEffort, + SaveThinkingSelection, + SelectionOption, + SetThinkingRequest, + ThinkingResponse, + ThinkingSaveResult, + ThinkingSelection, + ThinkingSelectionSaveResult, } from "./reasoning-effort"; export { DEFAULT_REASONING_EFFORT, effectiveEffort, + effectiveSelection, effortOptions, isReasoningEffort, + isThinkingSelection, REASONING_EFFORT_LEVELS, + selectionOptions, } from "./reasoning-effort"; export type { ChatStore, ChatStoreDependencies } from "./store.svelte"; export { createChatStore } from "./store.svelte"; diff --git a/src/features/chat/ports.ts b/src/features/chat/ports.ts index 2fe10dc..53ac236 100644 --- a/src/features/chat/ports.ts +++ b/src/features/chat/ports.ts @@ -1,4 +1,5 @@ import type { + ChatQueueCancelMessage, ChatQueueMessage, ChatSendMessage, ConversationHistoryResponse, @@ -6,12 +7,14 @@ import type { } from "@dispatch/transport-contract"; /** - * Injected transport port — sends chat messages to the server. Accepts both - * `chat.send` (start a turn) and `chat.queue` (enqueue a steering message; - * auto-starts a turn if idle). + * Injected transport port — sends chat messages to the server. Accepts + * `chat.send` (start a turn), `chat.queue` (enqueue a steering message; + * auto-starts a turn if idle), and `chat.queue.cancel` (remove a single queued + * message by id so it never runs — fire-and-forget, idempotent; the + * message-queue surface confirms the removal). */ export interface ChatTransport { - send(msg: ChatSendMessage | ChatQueueMessage): void; + send(msg: ChatSendMessage | ChatQueueMessage | ChatQueueCancelMessage): void; } /** diff --git a/src/features/chat/reasoning-effort.test.ts b/src/features/chat/reasoning-effort.test.ts index 6d409e9..e870bac 100644 --- a/src/features/chat/reasoning-effort.test.ts +++ b/src/features/chat/reasoning-effort.test.ts @@ -2,9 +2,12 @@ import { describe, expect, it } from "vitest"; import { DEFAULT_REASONING_EFFORT, effectiveEffort, + effectiveSelection, effortOptions, isReasoningEffort, + isThinkingSelection, REASONING_EFFORT_LEVELS, + selectionOptions, } from "./reasoning-effort"; describe("reasoning-effort helpers", () => { @@ -43,3 +46,39 @@ describe("reasoning-effort helpers", () => { } }); }); + +describe("thinking selection (the separate on/off axis)", () => { + it("selectionOptions lists 'off' first, then the ladder (default marked)", () => { + const options = selectionOptions(); + expect(options).toHaveLength(1 + REASONING_EFFORT_LEVELS.length); + expect(options[0]?.value).toBe("off"); + expect(options[0]?.label).toBe("Off"); + // the rest are the ladder, unchanged from effortOptions() + expect(options.slice(1).map((o) => o.value)).toEqual([...REASONING_EFFORT_LEVELS]); + expect(options.find((o) => o.value === "high")?.label).toBe("high (default)"); + }); + + it("isThinkingSelection narrows 'off' + ladder strings, rejects the rest", () => { + expect(isThinkingSelection("off")).toBe(true); + for (const level of REASONING_EFFORT_LEVELS) { + expect(isThinkingSelection(level)).toBe(true); + } + expect(isThinkingSelection("banana")).toBe(false); + expect(isThinkingSelection("")).toBe(false); + expect(isThinkingSelection("OFF")).toBe(false); + expect(isThinkingSelection("none")).toBe(false); // NOT a wire value we send + }); + + it("effectiveSelection shows 'off' when thinking is explicitly disabled", () => { + // thinking off is a SEPARATE axis: the effort level is irrelevant while off. + expect(effectiveSelection("xhigh", false)).toBe("off"); + expect(effectiveSelection(null, false)).toBe("off"); + }); + + it("effectiveSelection shows the effort level when thinking is on (default)", () => { + // null thinking = never set ⇒ thinking ON (default) ⇒ show the effort level. + expect(effectiveSelection(null, null)).toBe("high"); // default effort + expect(effectiveSelection("low", null)).toBe("low"); + expect(effectiveSelection("max", true)).toBe("max"); // explicitly on + }); +}); diff --git a/src/features/chat/reasoning-effort.ts b/src/features/chat/reasoning-effort.ts index 1eb77b6..39e1c5a 100644 --- a/src/features/chat/reasoning-effort.ts +++ b/src/features/chat/reasoning-effort.ts @@ -36,7 +36,7 @@ export function effectiveEffort(persisted: ReasoningEffort | null): ReasoningEff return persisted ?? DEFAULT_REASONING_EFFORT; } -/** One `<option>` of the selector. */ +/** One `<option>` of the effort ladder. */ export interface EffortOption { readonly value: ReasoningEffort; readonly label: string; @@ -64,3 +64,99 @@ export type ReasoningEffortSaveResult = export type SaveReasoningEffort = ( level: ReasoningEffort, ) => Promise<ReasoningEffortSaveResult | null>; + +// ── Thinking on/off (a SEPARATE axis from the effort level) ───────────────── +// +// Per the umans API (and the user's mental model), "thinking off" is NOT a +// zero-effort level — it is a distinct "disable extended thinking entirely" +// signal. The umans route expresses it as `reasoning_effort: "none"`; Dispatch +// surfaces it as a SEPARATE per-conversation boolean so the effort LEVEL is +// preserved across an off→on toggle (turning thinking back on restores the +// previously-chosen depth). The per-conversation selector CONFLATES the two +// axes into one `<select>` (UX), but the WIRE keeps them separate. +// +// ⚠️ BACKEND CONTRACT GAP — see `backend-handoff.md`. The `thinking` endpoint + +// wire types below are the PROPOSED shape; the backend has NOT shipped them yet. +// They are defined FE-local (mirroring the shipped `ReasoningEffortResponse` / +// `SetReasoningEffortRequest` shape) so the FE is built + tested against the +// target contract. Re-pin + re-mirror once the backend ships them. + +/** + * Response of `GET /conversations/:id/thinking` (PROPOSED). `thinking` is null + * when never set (the server then resolves turns with thinking ON — the + * default), `false` when explicitly disabled, `true` when explicitly enabled. + */ +export interface ThinkingResponse { + readonly conversationId: string; + readonly thinking: boolean | null; +} + +/** Body of `PUT /conversations/:id/thinking` (PROPOSED). */ +export interface SetThinkingRequest { + readonly thinking: boolean; +} + +/** + * The per-conversation selector's value: `"off"` (thinking disabled — the + * separate axis) or a reasoning-effort LEVEL. NOT a widened ladder: `"off"` is + * not a degree of effort, it is the absence of thinking. + */ +export type ThinkingSelection = "off" | ReasoningEffort; + +/** One `<option>` of the combined selector (off or a level). */ +export interface SelectionOption { + readonly value: ThinkingSelection; + readonly label: string; +} + +/** + * The selector's options: `"off"` first (the separate disable signal), then the + * effort ladder with the server default marked `(default)`. A never-set + * conversation (thinking on, effort null) reads "high (default)". + */ +export function selectionOptions(): readonly SelectionOption[] { + return [{ value: "off", label: "Off" }, ...effortOptions()]; +} + +/** Narrow an untrusted `<select>` value to a {@link ThinkingSelection}. */ +export function isThinkingSelection(value: string): value is ThinkingSelection { + return value === "off" || isReasoningEffort(value); +} + +/** + * The selection the per-conversation selector should show as selected: `"off"` + * when thinking is explicitly disabled (`persistedThinking === false`), else the + * effective effort level. `persistedThinking === null` (never set) ⇒ thinking + * ON (the default) ⇒ the effort level is shown — NOT "off". + */ +export function effectiveSelection( + persistedEffort: ReasoningEffort | null, + persistedThinking: boolean | null, +): ThinkingSelection { + if (persistedThinking === false) return "off"; + return effectiveEffort(persistedEffort); +} + +// ── Injected port for the combined selector (off OR a level) ───────────────── + +/** Outcome of persisting a {@link ThinkingSelection} (one or two PUTs). */ +export type ThinkingSelectionSaveResult = + | { readonly ok: true; readonly selection: ThinkingSelection } + | { readonly ok: false; readonly error: string }; + +/** + * Persist a thinking selection (consumer-defines-port; the composition root + * adapts the store's `PUT .../thinking` + `PUT .../reasoning-effort` to this). + * - `"off"` → disable thinking (the separate signal); the effort level is left + * untouched so an off→on toggle restores it. + * - a level → set the effort level AND ensure thinking is ON (the level is + * meaningless while thinking is off). + */ +export type SaveThinkingSelection = ( + selection: ThinkingSelection, +) => Promise<ThinkingSelectionSaveResult | null>; + +/** Outcome of `PUT /conversations/:id/thinking`. */ +export type ThinkingSaveResult = + | { readonly ok: true; readonly thinking: boolean } + | { readonly ok: false; readonly error: string }; diff --git a/src/features/chat/store.svelte.ts b/src/features/chat/store.svelte.ts index 9911438..24a1d06 100644 --- a/src/features/chat/store.svelte.ts +++ b/src/features/chat/store.svelte.ts @@ -1,6 +1,7 @@ import type { ChatDeltaMessage, ChatErrorMessage, + ChatQueueCancelMessage, ChatQueueMessage, ChatSendMessage, } from "@dispatch/transport-contract"; @@ -131,6 +132,17 @@ export interface ChatStore { * transcript. `text` must be non-empty (the server 400/errors otherwise). */ queueMessage(text: string): void; + /** + * Cancel (remove) a single queued steering message by id so it never runs + * (`chat.queue.cancel` WS op). Fire-and-forget + idempotent: success is + * confirmed by the `message-queue` SURFACE updating (the cancelled message + * leaves the snapshot); a cancel of an already-drained / unknown message is a + * silent server-side no-op. The caller optimistically hides the row; the + * surface update reconciles. `messageId` is the stable `QueuedMessage.id` + * the queue surface snapshot carries. No transcript change — a cancelled + * message is never delivered as steering. + */ + cancelQueuedMessage(messageId: string): void; setModel(model: string): void; /** * Update the chat limit LIVE: re-normalizes, then adjusts the loaded window. @@ -348,6 +360,21 @@ export function createChatStore(deps: ChatStoreDependencies): ChatStore { deps.transport.send(msg); }, + cancelQueuedMessage(messageId: string): void { + // Fire-and-forget + idempotent (per the contract). The caller optimistically + // hides the row; the message-queue surface update reconciles. A cancel of + // an already-drained / unknown message is a silent server no-op, so there + // is no local-state change to make and nothing to roll back on a stray + // `chat.error` (which only fires for a malformed send — a client that + // sends the id it just rendered never hits it). + const msg: ChatQueueCancelMessage = { + type: "chat.queue.cancel", + conversationId: deps.conversationId, + messageId, + }; + deps.transport.send(msg); + }, + setModel(model: string): void { _model = model; }, diff --git a/src/features/chat/store.test.ts b/src/features/chat/store.test.ts index 8f36994..aa5560f 100644 --- a/src/features/chat/store.test.ts +++ b/src/features/chat/store.test.ts @@ -318,6 +318,78 @@ describe("createChatStore", () => { }); }); + describe("cancelQueuedMessage (chat.queue.cancel)", () => { + it("posts a chat.queue.cancel with conversationId + messageId", () => { + const transport = createFakeTransport(); + const historySync = createFakeHistorySync(); + const metricsSync = createFakeMetricsSync(); + const cache = createFakeCache(); + const store = createChatStore({ + conversationId: CONV_ID, + transport: transport.impl, + historySync: historySync.impl, + metricsSync: metricsSync.impl, + cache: cache.impl, + }); + + store.cancelQueuedMessage("msg-42"); + + expect(transport.sent).toHaveLength(0); // chat.send stays empty + expect(transport.sentQueue).toHaveLength(0); // chat.queue stays empty + expect(transport.sentCancels).toHaveLength(1); + expect(transport.sentCancels[0]?.type).toBe("chat.queue.cancel"); + expect(transport.sentCancels[0]?.conversationId).toBe(CONV_ID); + expect(transport.sentCancels[0]?.messageId).toBe("msg-42"); + + store.dispose(); + }); + + it("does NOT touch the transcript (a cancelled message never runs)", () => { + const transport = createFakeTransport(); + const historySync = createFakeHistorySync(); + const metricsSync = createFakeMetricsSync(); + const cache = createFakeCache(); + const store = createChatStore({ + conversationId: CONV_ID, + transport: transport.impl, + historySync: historySync.impl, + metricsSync: metricsSync.impl, + cache: cache.impl, + }); + + store.cancelQueuedMessage("msg-42"); + + expect(store.chunks).toHaveLength(0); // no transcript echo / change + expect(store.error).toBeNull(); + + store.dispose(); + }); + + it("sends for any messageId (cancel is idempotent server-side, no FE guard)", () => { + const transport = createFakeTransport(); + const historySync = createFakeHistorySync(); + const metricsSync = createFakeMetricsSync(); + const cache = createFakeCache(); + const store = createChatStore({ + conversationId: CONV_ID, + transport: transport.impl, + historySync: historySync.impl, + metricsSync: metricsSync.impl, + cache: cache.impl, + }); + + // The server no-ops an already-drained / unknown id; the FE fires-and- + // forgetgets, so even a repeat cancel is forwarded. + store.cancelQueuedMessage("msg-42"); + store.cancelQueuedMessage("msg-42"); + + expect(transport.sentCancels).toHaveLength(2); + expect(transport.sentCancels[1]?.messageId).toBe("msg-42"); + + store.dispose(); + }); + }); + it("chat.error sets error", () => { const transport = createFakeTransport(); const historySync = createFakeHistorySync(); diff --git a/src/features/chat/test-helpers.ts b/src/features/chat/test-helpers.ts index 26c5590..c99d1f4 100644 --- a/src/features/chat/test-helpers.ts +++ b/src/features/chat/test-helpers.ts @@ -1,4 +1,8 @@ -import type { ChatQueueMessage, ChatSendMessage } from "@dispatch/transport-contract"; +import type { + ChatQueueCancelMessage, + ChatQueueMessage, + ChatSendMessage, +} from "@dispatch/transport-contract"; import type { StoredChunk } from "@dispatch/wire"; import type { ConversationCache } from "../conversation-cache"; import type { ChatTransport, HistorySync, HistoryWindow, MetricsSync } from "./ports"; @@ -8,19 +12,25 @@ export interface FakeTransport { readonly sent: ChatSendMessage[]; /** All `chat.queue` messages sent through the fake transport. */ readonly sentQueue: ChatQueueMessage[]; + /** All `chat.queue.cancel` messages sent through the fake transport. */ + readonly sentCancels: ChatQueueCancelMessage[]; readonly impl: ChatTransport; } export function createFakeTransport(): FakeTransport { const sent: ChatSendMessage[] = []; const sentQueue: ChatQueueMessage[] = []; + const sentCancels: ChatQueueCancelMessage[] = []; return { sent, sentQueue, + sentCancels, impl: { send(msg) { if (msg.type === "chat.queue") { sentQueue.push(msg); + } else if (msg.type === "chat.queue.cancel") { + sentCancels.push(msg); } else { sent.push(msg); } diff --git a/src/features/chat/ui.test.ts b/src/features/chat/ui.test.ts index 5f8067d..f4006f7 100644 --- a/src/features/chat/ui.test.ts +++ b/src/features/chat/ui.test.ts @@ -1092,30 +1092,53 @@ describe("ModelSelector", () => { }); describe("ReasoningEffortSelector", () => { - it("renders null (never set) as the default level, marked '(default)'", () => { - render(ReasoningEffortSelector, { props: { persisted: null, save: vi.fn() } }); + it("renders null effort + null thinking (never set) as the default level, marked '(default)'", () => { + render(ReasoningEffortSelector, { + props: { persistedEffort: null, persistedThinking: null, save: vi.fn() }, + }); const select = screen.getByRole("combobox", { name: "Reasoning effort" }); expect(select).toHaveValue("high"); expect(within(select).getByRole("option", { name: "high (default)" })).toBeInTheDocument(); - // All five ladder levels are offered. - expect(within(select).getAllByRole("option")).toHaveLength(5); + // "Off" first, then the five ladder levels. + const options = within(select).getAllByRole("option"); + expect(options).toHaveLength(6); + expect(options[0]).toHaveValue("off"); + expect(within(select).getByRole("option", { name: "Off" })).toBeInTheDocument(); }); - it("renders a persisted level as selected", () => { - render(ReasoningEffortSelector, { props: { persisted: "xhigh", save: vi.fn() } }); + it("renders a persisted level as selected when thinking is on", () => { + render(ReasoningEffortSelector, { + props: { persistedEffort: "xhigh", persistedThinking: null, save: vi.fn() }, + }); expect(screen.getByRole("combobox", { name: "Reasoning effort" })).toHaveValue("xhigh"); }); + it("renders 'off' as selected when thinking is disabled (effort level is preserved but hidden)", () => { + // thinking off is a SEPARATE axis: even with a persisted effort level, the + // selector shows "off" while thinking is disabled. + render(ReasoningEffortSelector, { + props: { persistedEffort: "xhigh", persistedThinking: false, save: vi.fn() }, + }); + + expect(screen.getByRole("combobox", { name: "Reasoning effort" })).toHaveValue("off"); + // the level option is still present (restored on an off→on toggle) + expect( + within(screen.getByRole("combobox")).getByRole("option", { name: "xhigh" }), + ).toBeInTheDocument(); + }); + it("selecting a level saves it via the injected port and confirms", async () => { - const save = vi.fn(async (level: "low" | "medium" | "high" | "xhigh" | "max") => ({ + const save = vi.fn(async (selection: "off" | "low" | "medium" | "high" | "xhigh" | "max") => ({ ok: true as const, - reasoningEffort: level, + selection, })); const user = userEvent.setup(); - render(ReasoningEffortSelector, { props: { persisted: null, save } }); + render(ReasoningEffortSelector, { + props: { persistedEffort: null, persistedThinking: null, save }, + }); await user.selectOptions(screen.getByRole("combobox", { name: "Reasoning effort" }), "max"); @@ -1127,11 +1150,35 @@ describe("ReasoningEffortSelector", () => { expect(screen.getByRole("combobox", { name: "Reasoning effort" })).toHaveValue("max"); }); - it("a failed save shows the error and reverts to the persisted value", async () => { + it("selecting 'off' saves the separate disable signal (not a level)", async () => { + const save = vi.fn(async (selection: "off" | "low" | "medium" | "high" | "xhigh" | "max") => ({ + ok: true as const, + selection, + })); + const user = userEvent.setup(); + + render(ReasoningEffortSelector, { + props: { persistedEffort: "high", persistedThinking: null, save }, + }); + + await user.selectOptions(screen.getByRole("combobox", { name: "Reasoning effort" }), "off"); + + expect(save).toHaveBeenCalledTimes(1); + // "off" is the separate thinking-disable signal — NOT a zero-effort level. + expect(save).toHaveBeenCalledWith("off"); + await vi.waitFor(() => { + expect(screen.getByText(/applies from the next turn/i)).toBeInTheDocument(); + }); + expect(screen.getByRole("combobox", { name: "Reasoning effort" })).toHaveValue("off"); + }); + + it("a failed save shows the error and reverts to the persisted selection", async () => { const save = vi.fn(async () => ({ ok: false as const, error: "nope" })); const user = userEvent.setup(); - render(ReasoningEffortSelector, { props: { persisted: "low", save } }); + render(ReasoningEffortSelector, { + props: { persistedEffort: "low", persistedThinking: null, save }, + }); await user.selectOptions(screen.getByRole("combobox", { name: "Reasoning effort" }), "max"); @@ -1142,22 +1189,24 @@ describe("ReasoningEffortSelector", () => { }); it("disables the select while a save is in flight (no double-fire)", async () => { - let resolveSave: ((r: { ok: true; reasoningEffort: "max" }) => void) | undefined; + let resolveSave: ((r: { ok: true; selection: "max" }) => void) | undefined; const save = vi.fn( () => - new Promise<{ ok: true; reasoningEffort: "max" }>((resolve) => { + new Promise<{ ok: true; selection: "max" }>((resolve) => { resolveSave = resolve; }), ); const user = userEvent.setup(); - render(ReasoningEffortSelector, { props: { persisted: null, save } }); + render(ReasoningEffortSelector, { + props: { persistedEffort: null, persistedThinking: null, save }, + }); await user.selectOptions(screen.getByRole("combobox", { name: "Reasoning effort" }), "max"); expect(screen.getByRole("combobox", { name: "Reasoning effort" })).toBeDisabled(); - resolveSave?.({ ok: true, reasoningEffort: "max" }); + resolveSave?.({ ok: true, selection: "max" }); await vi.waitFor(() => { expect(screen.getByRole("combobox", { name: "Reasoning effort" })).toBeEnabled(); }); diff --git a/src/features/chat/ui/ReasoningEffortSelector.svelte b/src/features/chat/ui/ReasoningEffortSelector.svelte index d982905..6858779 100644 --- a/src/features/chat/ui/ReasoningEffortSelector.svelte +++ b/src/features/chat/ui/ReasoningEffortSelector.svelte @@ -1,34 +1,45 @@ <script lang="ts"> import type { ReasoningEffort } from "@dispatch/transport-contract"; import { - effectiveEffort, - effortOptions, - isReasoningEffort, - type SaveReasoningEffort, + effectiveSelection, + isThinkingSelection, + selectionOptions, + type SaveThinkingSelection, + type ThinkingSelection, } from "../reasoning-effort"; let { - persisted, + persistedEffort, + persistedThinking, save, }: { - /** The conversation's persisted level, or null when never set (default applies). */ - persisted: ReasoningEffort | null; - save: SaveReasoningEffort; + /** The conversation's persisted effort level, or null when never set (default applies). */ + persistedEffort: ReasoningEffort | null; + /** + * The conversation's persisted thinking flag, or null when never set + * (thinking ON — the default). `false` ⇒ thinking disabled (the separate + * "off" axis); the effort level is preserved across an off→on toggle. + */ + persistedThinking: boolean | null; + /** Persist a thinking selection (off or a level). */ + save: SaveThinkingSelection; } = $props(); - const options = effortOptions(); + const options = selectionOptions(); - // The user's in-flight choice; null = mirror the (async-loaded) persisted prop. - // Re-mounted per conversation, so there is no cross-tab bleed. - let chosen = $state<ReasoningEffort | null>(null); + // The user's in-flight choice; null = mirror the (async-loaded) persisted + // selection. Re-mounted per conversation, so there is no cross-tab bleed. + let chosen = $state<ThinkingSelection | null>(null); let saving = $state(false); let error = $state<string | null>(null); let justSaved = $state(false); - const selected = $derived(chosen ?? effectiveEffort(persisted)); + const selected = $derived( + chosen ?? effectiveSelection(persistedEffort, persistedThinking), + ); async function handleChange(value: string) { - if (!isReasoningEffort(value) || saving) return; + if (!isThinkingSelection(value) || saving) return; chosen = value; saving = true; error = null; @@ -40,7 +51,7 @@ justSaved = true; } else { error = result.error; - chosen = null; // revert to the persisted value + chosen = null; // revert to the persisted selection } } </script> @@ -69,7 +80,7 @@ <p class="text-xs text-success">Saved — applies from the next turn.</p> {:else} <p class="text-xs opacity-50"> - How long the model thinks before answering. Changing it can re-prefill the prompt cache once. + How long the model thinks before answering. “Off” disables thinking entirely. Changing it can re-prefill the prompt cache once. </p> {/if} </div> diff --git a/src/features/heartbeat/logic/types.ts b/src/features/heartbeat/logic/types.ts index 3d3d525..83cec74 100644 --- a/src/features/heartbeat/logic/types.ts +++ b/src/features/heartbeat/logic/types.ts @@ -26,6 +26,15 @@ export type HeartbeatRunStatus = "running" | "completed" | "stopped"; export interface HeartbeatConfig { /** Whether the autonomous loop is enabled (running on the interval). */ readonly enabled: boolean; + /** + * When true (the default), the heartbeat SKIPS a fire whenever the configured + * workspace has any active agents (a conversation whose persisted status is + * `"active"` or `"queued"`) — it stays quiet while the user is actively + * working and only fires when the workspace is idle. When false, the heartbeat + * fires unconditionally on every interval. The heartbeat-spawned conversation + * lives in a dedicated workspace, so an in-flight run never self-blocks. + */ + readonly inactiveOnly: boolean; readonly systemPrompt: string; readonly taskPrompt: string; /** Minutes between runs. */ @@ -46,6 +55,7 @@ export interface HeartbeatConfig { */ export interface HeartbeatConfigPatch { readonly enabled?: boolean; + readonly inactiveOnly?: boolean; readonly systemPrompt?: string; readonly taskPrompt?: string; readonly intervalMinutes?: number; diff --git a/src/features/heartbeat/logic/view-model.test.ts b/src/features/heartbeat/logic/view-model.test.ts index aca0aa6..c9ef118 100644 --- a/src/features/heartbeat/logic/view-model.test.ts +++ b/src/features/heartbeat/logic/view-model.test.ts @@ -39,6 +39,7 @@ const run = (over: Partial<HeartbeatRun> = {}): HeartbeatRun => ({ const config = (over: Partial<HeartbeatConfig> = {}): HeartbeatConfig => ({ enabled: false, + inactiveOnly: true, systemPrompt: "be helpful", taskPrompt: "check status", intervalMinutes: 15, @@ -254,6 +255,40 @@ describe("config form", () => { const f = formFromConfig(c); expect(formDiffers(f, c)).toBe(false); // null resolves to "high" == form }); + + it("emptyForm defaults inactiveOnly to true (on by default)", () => { + expect(emptyForm().inactiveOnly).toBe(true); + }); + + it("formFromConfig carries inactiveOnly through verbatim", () => { + expect(formFromConfig(config({ inactiveOnly: true })).inactiveOnly).toBe(true); + expect(formFromConfig(config({ inactiveOnly: false })).inactiveOnly).toBe(false); + }); + + it("formFromConfig coerces a missing/malformed inactiveOnly to the default (true)", () => { + // A legacy config (undefined) or a non-boolean is read as ON (true) — matches + // normalizeHeartbeatConfig's default and the backend's "on by default". + const f = formFromConfig(config({ inactiveOnly: undefined as unknown as boolean })); + expect(f.inactiveOnly).toBe(true); + }); + + it("patchFromForm carries inactiveOnly", () => { + expect(patchFromForm(formFromConfig(config({ inactiveOnly: false }))).inactiveOnly).toBe(false); + expect(patchFromForm(formFromConfig(config({ inactiveOnly: true }))).inactiveOnly).toBe(true); + }); + + it("formDiffers is true after toggling inactiveOnly", () => { + const c = config({ inactiveOnly: true }); + const f = formFromConfig(c); + f.inactiveOnly = false; + expect(formDiffers(f, c)).toBe(true); + }); + + it("formDiffers is false for a form seeded from the config (inactiveOnly unchanged)", () => { + const c = config({ inactiveOnly: false }); + const f = formFromConfig(c); + expect(formDiffers(f, c)).toBe(false); + }); }); describe("system-prompt inheritance (override ⇄ global default)", () => { @@ -321,6 +356,7 @@ describe("normalizeHeartbeatConfig", () => { it("passes through a well-formed config", () => { const c = normalizeHeartbeatConfig({ enabled: true, + inactiveOnly: false, systemPrompt: "sys", taskPrompt: "task", intervalMinutes: 20, @@ -329,6 +365,7 @@ describe("normalizeHeartbeatConfig", () => { }); expect(c).toEqual({ enabled: true, + inactiveOnly: false, systemPrompt: "sys", taskPrompt: "task", intervalMinutes: 20, @@ -339,10 +376,12 @@ describe("normalizeHeartbeatConfig", () => { it("coerces a malformed body safely (never throws, never undefined)", () => { const c = normalizeHeartbeatConfig({ enabled: "yes", + inactiveOnly: "yes", intervalMinutes: -3, reasoningEffort: "bogus", }); expect(c.enabled).toBe(false); + expect(c.inactiveOnly).toBe(true); // non-boolean → default ON expect(c.intervalMinutes).toBe(1); expect(c.reasoningEffort).toBeNull(); expect(c.systemPrompt).toBe(""); @@ -355,12 +394,30 @@ describe("normalizeHeartbeatConfig", () => { it("handles null / non-object input", () => { const c = normalizeHeartbeatConfig(null); expect(c.enabled).toBe(false); + expect(c.inactiveOnly).toBe(true); // default ON for an absent config expect(c.intervalMinutes).toBe(DEFAULT_INTERVAL_MINUTES); expect(c.model).toBe(""); }); it("clamps a huge interval", () => { expect(normalizeHeartbeatConfig({ intervalMinutes: 99999 }).intervalMinutes).toBe(1440); }); + it("inactiveOnly defaults to true when absent (legacy config → on by default)", () => { + // A config persisted by an older backend (no inactiveOnly field) reads back + // as true — the feature is ON by default for everyone. + expect(normalizeHeartbeatConfig({}).inactiveOnly).toBe(true); + expect(normalizeHeartbeatConfig({ inactiveOnly: undefined }).inactiveOnly).toBe(true); + }); + it("inactiveOnly passes through an explicit false (opt-out)", () => { + expect(normalizeHeartbeatConfig({ inactiveOnly: false }).inactiveOnly).toBe(false); + expect(normalizeHeartbeatConfig({ inactiveOnly: true }).inactiveOnly).toBe(true); + }); + it("inactiveOnly treats only an explicit boolean false as false (not 0, not null)", () => { + // The wire contract requires a JSON boolean; a non-boolean (0, null, "no") + // is treated as the default (true) rather than silently misbehaving. + expect(normalizeHeartbeatConfig({ inactiveOnly: 0 }).inactiveOnly).toBe(true); + expect(normalizeHeartbeatConfig({ inactiveOnly: null }).inactiveOnly).toBe(true); + expect(normalizeHeartbeatConfig({ inactiveOnly: "false" }).inactiveOnly).toBe(true); + }); }); describe("normalizeHeartbeatRuns", () => { diff --git a/src/features/heartbeat/logic/view-model.ts b/src/features/heartbeat/logic/view-model.ts index e91febd..4a5ba7f 100644 --- a/src/features/heartbeat/logic/view-model.ts +++ b/src/features/heartbeat/logic/view-model.ts @@ -220,6 +220,7 @@ export function approximateNextRunEpoch( */ export interface HeartbeatFormState { enabled: boolean; + inactiveOnly: boolean; systemPrompt: string; taskPrompt: string; intervalHours: number; @@ -254,6 +255,7 @@ export function formFromConfig(config: HeartbeatConfig): HeartbeatFormState { const { hours, minutes } = splitInterval(config.intervalMinutes); return { enabled: config.enabled === true, + inactiveOnly: config.inactiveOnly !== false, systemPrompt: config.systemPrompt ?? "", taskPrompt: config.taskPrompt ?? "", intervalHours: hours, @@ -268,6 +270,7 @@ export function emptyForm(): HeartbeatFormState { const { hours, minutes } = splitInterval(DEFAULT_INTERVAL_MINUTES); return { enabled: false, + inactiveOnly: true, systemPrompt: "", taskPrompt: "", intervalHours: hours, @@ -295,6 +298,7 @@ export function normalizeInterval(value: unknown): number { export function patchFromForm(form: HeartbeatFormState): HeartbeatConfigPatch { return { enabled: form.enabled, + inactiveOnly: form.inactiveOnly, systemPrompt: form.systemPrompt, taskPrompt: form.taskPrompt, intervalMinutes: joinInterval(form.intervalHours, form.intervalMinutes), @@ -308,6 +312,7 @@ export function formDiffers(form: HeartbeatFormState, config: HeartbeatConfig): const { hours, minutes } = splitInterval(config.intervalMinutes); return ( form.enabled !== config.enabled || + form.inactiveOnly !== config.inactiveOnly || form.systemPrompt !== (config.systemPrompt ?? "") || form.taskPrompt !== (config.taskPrompt ?? "") || form.intervalHours !== hours || @@ -390,6 +395,10 @@ export function normalizeHeartbeatConfig(data: unknown): HeartbeatConfig { const effort = d.reasoningEffort; return { enabled: d.enabled === true, + // Default ON (true): a missing/falsey-but-not-false field (a legacy config + // persisted before the field shipped) reads back as inactiveOnly: true — + // the feature is on by default for everyone. Only an explicit `false` opts out. + inactiveOnly: d.inactiveOnly !== false, systemPrompt: typeof d.systemPrompt === "string" ? d.systemPrompt : "", taskPrompt: typeof d.taskPrompt === "string" ? d.taskPrompt : "", intervalMinutes: normalizeInterval(d.intervalMinutes), diff --git a/src/features/heartbeat/ui/HeartbeatView.svelte b/src/features/heartbeat/ui/HeartbeatView.svelte index 5f262f8..7f95c40 100644 --- a/src/features/heartbeat/ui/HeartbeatView.svelte +++ b/src/features/heartbeat/ui/HeartbeatView.svelte @@ -140,6 +140,31 @@ } } + // The inactive-only checkbox is a save-on-change control (like the enable + // toggle): a partial PUT { inactiveOnly } — no need to round-trip the rest + // of the form. The heartbeat then skips a fire whenever the workspace has + // active agents (a conversation whose status is "active" or "queued"). + async function handleToggleInactiveOnly(): Promise<void> { + if (saving) return; + const next = !form.inactiveOnly; + form = { ...form, inactiveOnly: next }; + saving = true; + saveError = null; + justSaved = false; + const result = await saveConfig({ inactiveOnly: next }); + saving = false; + if (result === null) return; + if (result.ok) { + form = formFromConfig(result.config); + loadedConfig = formFromConfig(result.config); + justSaved = true; + } else { + saveError = result.error; + // Revert the checkbox to the last-known state. + form = { ...form, inactiveOnly: loadedConfig.inactiveOnly }; + } + } + // ── Runs list (polls while mounted) ─────────────────────────────────────── let runs = $state<readonly HeartbeatRunView[]>([]); /** The raw backend runs (carry `triggeredAt`), kept for the next-run @@ -323,6 +348,27 @@ {#if configError} <p class="text-xs text-error">{configError}</p> {:else} + <!-- Inactive-only (skip fires while the workspace has active agents) --> + <section class="flex flex-col gap-1"> + <label class="flex items-start gap-2 text-sm"> + <input + type="checkbox" + class="checkbox checkbox-sm checkbox-primary mt-0.5" + checked={form.inactiveOnly} + disabled={saving || configLoading} + onchange={handleToggleInactiveOnly} + aria-label="Only run the heartbeat when the workspace is idle" + /> + <span class="flex flex-col gap-0.5"> + <span>Only run when idle</span> + <span class="text-xs opacity-50"> + Skip heartbeat fires while agents are active in this workspace. When off, the + heartbeat runs on every interval regardless of activity. + </span> + </span> + </label> + </section> + <!-- Prompts (open the full-page editor) --> <section class="flex flex-col gap-1"> <span class="text-xs font-semibold uppercase opacity-60">Prompts</span> diff --git a/src/features/heartbeat/ui/HeartbeatView.test.ts b/src/features/heartbeat/ui/HeartbeatView.test.ts new file mode 100644 index 0000000..89eeee3 --- /dev/null +++ b/src/features/heartbeat/ui/HeartbeatView.test.ts @@ -0,0 +1,203 @@ +import { render, screen } from "@testing-library/svelte"; +import userEvent from "@testing-library/user-event"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { LoadSystemPrompt, LoadSystemPromptVariables } from "../../system-prompt"; +import type { + HeartbeatConfig, + HeartbeatConfigPatch, + HeartbeatConfigResult, + HeartbeatNextRunResult, + HeartbeatStopResult, + LoadHeartbeatConfig, + LoadHeartbeatNextRun, + LoadHeartbeatRuns, + SaveHeartbeatConfig, + StopHeartbeatRun, +} from "../logic/types"; +import HeartbeatView from "./HeartbeatView.svelte"; + +// ── Fakes for the injected ports ───────────────────────────────────────────── +// Only the OUTERMOST edges are faked (the save/load ports); no sibling module is +// mocked. Mirrors the PromptEditor test's fake-port pattern. + +function makeConfig(over: Partial<HeartbeatConfig> = {}): HeartbeatConfig { + return { + enabled: false, + inactiveOnly: true, + systemPrompt: "", + taskPrompt: "", + intervalMinutes: 30, + model: "", + reasoningEffort: null, + ...over, + }; +} + +/** A capturing saveConfig that echoes a merged config (so the form re-seeds). */ +function fakeSaveConfig(initial: HeartbeatConfig): { + calls: HeartbeatConfigPatch[]; + impl: SaveHeartbeatConfig; +} { + const calls: HeartbeatConfigPatch[] = []; + let current = initial; + const impl: SaveHeartbeatConfig = async (patch) => { + calls.push(patch); + // Echo the merged config so the component re-seeds from the server response. + current = { ...current, ...patch }; + return { ok: true, config: current } satisfies HeartbeatConfigResult; + }; + return { calls, impl }; +} + +function fakeLoadConfig(config: HeartbeatConfig): LoadHeartbeatConfig { + return vi.fn(async () => ({ ok: true, config }) as const); +} + +function fakeLoadRuns(): LoadHeartbeatRuns { + return vi.fn(async () => ({ ok: true, runs: [] }) as const); +} + +function fakeStopRun(): StopHeartbeatRun { + return vi.fn(async () => ({ ok: true }) as const satisfies HeartbeatStopResult); +} + +function fakeLoadNextRun(): LoadHeartbeatNextRun { + // No scheduled run (heartbeat disabled in the default config) → no countdown. + return vi.fn( + async () => ({ ok: true, nextRunAt: null }) as const satisfies HeartbeatNextRunResult, + ); +} + +function fakeLoadVariables(): LoadSystemPromptVariables { + return vi.fn(async () => ({ ok: true, variables: [] }) as const); +} + +function fakeLoadDefaultPrompt(): LoadSystemPrompt { + return vi.fn(async () => ({ ok: true, template: "" }) as const); +} + +const baseProps = (overrides: Record<string, unknown> = {}) => ({ + models: [] as readonly string[], + loadConfig: fakeLoadConfig(makeConfig()), + saveConfig: fakeSaveConfig(makeConfig()).impl, + loadVariables: fakeLoadVariables(), + loadDefaultPrompt: fakeLoadDefaultPrompt(), + loadRuns: fakeLoadRuns(), + stopRun: fakeStopRun(), + loadNextRun: fakeLoadNextRun(), + onOpenRun: vi.fn(), + ...overrides, +}); + +// HeartbeatView sets up polling intervals (runs + next-run + clock) on mount. +// Clear any stray timers between tests so a later test never hangs on a leaked +// interval (the $effect cleanup clears them on unmount; this is belt+suspenders). +afterEach(() => { + vi.clearAllTimers(); +}); + +describe("HeartbeatView — inactive-only checkbox", () => { + it("renders checked when the loaded config has inactiveOnly: true (the default)", async () => { + const loadConfig = fakeLoadConfig(makeConfig({ inactiveOnly: true })); + render(HeartbeatView, { + props: baseProps({ loadConfig }), + }); + + const checkbox = await screen.findByLabelText( + "Only run the heartbeat when the workspace is idle", + ); + expect(checkbox).toBeChecked(); + }); + + it("renders unchecked when the loaded config has inactiveOnly: false", async () => { + const loadConfig = fakeLoadConfig(makeConfig({ inactiveOnly: false })); + render(HeartbeatView, { + props: baseProps({ loadConfig }), + }); + + const checkbox = await screen.findByLabelText( + "Only run the heartbeat when the workspace is idle", + ); + expect(checkbox).not.toBeChecked(); + }); + + it("toggling the checkbox persists a PARTIAL patch { inactiveOnly } and re-seeds", async () => { + const user = userEvent.setup(); + const initial = makeConfig({ inactiveOnly: true }); + const save = fakeSaveConfig(initial); + const loadConfig = fakeLoadConfig(initial); + render(HeartbeatView, { + props: baseProps({ loadConfig, saveConfig: save.impl }), + }); + + const checkbox = await screen.findByLabelText( + "Only run the heartbeat when the workspace is idle", + ); + expect(checkbox).toBeChecked(); + + await user.click(checkbox); + + // The save port was called with ONLY { inactiveOnly: false } — a partial + // update, not the whole config (mirrors the enable toggle's partial PUT). + await vi.waitFor(() => { + expect(save.calls).toHaveLength(1); + }); + expect(save.calls[0]).toEqual({ inactiveOnly: false }); + + // After the save resolves, the checkbox reflects the server response (unchecked). + await vi.waitFor(() => { + expect(checkbox).not.toBeChecked(); + }); + }); + + it("toggling back on sends { inactiveOnly: true }", async () => { + const user = userEvent.setup(); + const initial = makeConfig({ inactiveOnly: false }); + const save = fakeSaveConfig(initial); + const loadConfig = fakeLoadConfig(initial); + render(HeartbeatView, { + props: baseProps({ loadConfig, saveConfig: save.impl }), + }); + + const checkbox = await screen.findByLabelText( + "Only run the heartbeat when the workspace is idle", + ); + expect(checkbox).not.toBeChecked(); + + await user.click(checkbox); + + await vi.waitFor(() => { + expect(save.calls).toHaveLength(1); + }); + expect(save.calls[0]).toEqual({ inactiveOnly: true }); + await vi.waitFor(() => { + expect(checkbox).toBeChecked(); + }); + }); + + it("a failed save reverts the checkbox to the last-known state", async () => { + const user = userEvent.setup(); + const initial = makeConfig({ inactiveOnly: true }); + const failingSave: SaveHeartbeatConfig = async () => ({ + ok: false, + error: "boom", + }); + const loadConfig = fakeLoadConfig(initial); + render(HeartbeatView, { + props: baseProps({ loadConfig, saveConfig: failingSave }), + }); + + const checkbox = await screen.findByLabelText( + "Only run the heartbeat when the workspace is idle", + ); + expect(checkbox).toBeChecked(); + + await user.click(checkbox); + + // The failed save surfaces the error AND reverts the checkbox (stays checked). + await vi.waitFor(() => { + expect(screen.getByText("boom")).toBeInTheDocument(); + }); + expect(checkbox).toBeChecked(); + }); +}); diff --git a/src/features/heartbeat/ui/PromptEditor.test.ts b/src/features/heartbeat/ui/PromptEditor.test.ts index 284b319..c4bd3b8 100644 --- a/src/features/heartbeat/ui/PromptEditor.test.ts +++ b/src/features/heartbeat/ui/PromptEditor.test.ts @@ -29,6 +29,7 @@ function fakeSaveConfig(): { // Echo a config that reflects the persisted patch (so onSaved sync is realistic). const config = { enabled: false, + inactiveOnly: true, systemPrompt: patch.systemPrompt ?? "", taskPrompt: patch.taskPrompt ?? "", intervalMinutes: 30, diff --git a/src/features/surface-host/logic/message-queue.test.ts b/src/features/surface-host/logic/message-queue.test.ts index 8d55eb7..ae91c8f 100644 --- a/src/features/surface-host/logic/message-queue.test.ts +++ b/src/features/surface-host/logic/message-queue.test.ts @@ -1,6 +1,10 @@ import type { QueuedMessage } from "@dispatch/wire"; import { describe, expect, it } from "vitest"; -import { parseMessageQueuePayload } from "./message-queue"; +import { + parseMessageQueuePayload, + reconcileCancelledIds, + selectVisibleMessages, +} from "./message-queue"; const msg = (id: string, text: string, queuedAt = 1_700_000_000_000): QueuedMessage => ({ id, @@ -46,3 +50,56 @@ describe("parseMessageQueuePayload", () => { expect(parseMessageQueuePayload(payload)).toBeNull(); }); }); + +describe("selectVisibleMessages", () => { + it("returns the snapshot unchanged when nothing is cancelled", () => { + const messages = [msg("m1", "a"), msg("m2", "b")]; + expect(selectVisibleMessages(messages, new Set())).toBe(messages); + }); + + it("hides the optimistically-cancelled row", () => { + const messages = [msg("m1", "a"), msg("m2", "b"), msg("m3", "c")]; + expect(selectVisibleMessages(messages, new Set(["m2"])).map((m) => m.id)).toEqual(["m1", "m3"]); + }); + + it("hides multiple cancelled rows", () => { + const messages = [msg("m1", "a"), msg("m2", "b"), msg("m3", "c")]; + expect(selectVisibleMessages(messages, new Set(["m1", "m3"])).map((m) => m.id)).toEqual(["m2"]); + }); + + it("tolerates a cancelled id not present in the snapshot (no-op)", () => { + const messages = [msg("m1", "a")]; + expect(selectVisibleMessages(messages, new Set(["ghost"])).map((m) => m.id)).toEqual(["m1"]); + }); +}); + +describe("reconcileCancelledIds", () => { + it("returns an empty set when nothing was cancelled", () => { + const result = reconcileCancelledIds([msg("m1", "a")], new Set()); + expect(result.size).toBe(0); + }); + + it("drops ids the surface confirmed gone (no longer queued)", () => { + // m1 still queued (cancel pending), m2 confirmed gone (left the snapshot). + const messages = [msg("m1", "a")]; + const result = reconcileCancelledIds(messages, new Set(["m1", "m2"])); + expect([...result]).toEqual(["m1"]); + }); + + it("returns the SAME set identity when nothing changed (no spurious cycle)", () => { + const messages = [msg("m1", "a"), msg("m2", "b")]; + const cancelled = new Set(["m1", "m2"]); + expect(reconcileCancelledIds(messages, cancelled)).toBe(cancelled); + }); + + it("returns an empty set when all cancels were confirmed", () => { + const messages = [msg("m1", "a")]; + expect(reconcileCancelledIds(messages, new Set(["m2", "m3"])).size).toBe(0); + }); + + it("keeps a still-queued cancelled id (cancel still pending)", () => { + const messages = [msg("m1", "a"), msg("m2", "b")]; + const result = reconcileCancelledIds(messages, new Set(["m2"])); + expect([...result]).toEqual(["m2"]); + }); +}); diff --git a/src/features/surface-host/logic/message-queue.ts b/src/features/surface-host/logic/message-queue.ts index 79707a5..7a3653e 100644 --- a/src/features/surface-host/logic/message-queue.ts +++ b/src/features/surface-host/logic/message-queue.ts @@ -43,3 +43,64 @@ export function parseMessageQueuePayload(payload: unknown): MessageQueueData | n /** The `rendererId` the message-queue extension's `custom` surface field uses. */ export const MESSAGE_QUEUE_RENDERER_ID = "message-queue"; + +/** + * Optimistic-removal view-model for the queue list. + * + * The `chat.queue.cancel` op is fire-and-forget + idempotent: success is + * confirmed by the `message-queue` SURFACE updating (the cancelled message + * leaves the snapshot), not by a reply. To avoid a flash of the row lingering + * for a round-trip, the renderer hides a row the instant the user clicks cancel + * (tracking the cancelled id locally), then reconciles when the surface pushes + * the post-cancel snapshot. These two pure helpers drive that — the component + * holds the cancelled-id set as a thin `$state` wrapper and delegates all + * decisions here. + */ + +/** + * The messages the renderer should show: the surface snapshot MINUS any + * optimistically-cancelled ids (a cancel whose surface confirmation hasn't + * arrived yet). Pure — no mutation of inputs. + */ +export function selectVisibleMessages( + messages: readonly QueuedMessage[], + cancelledIds: ReadonlySet<string>, +): readonly QueuedMessage[] { + if (cancelledIds.size === 0) return messages; + return messages.filter((m) => !cancelledIds.has(m.id)); +} + +/** + * Reconcile the cancelled-id set against a NEW surface snapshot: keep only the + * ids that are STILL queued (the cancel is pending — its surface confirmation + * hasn't landed). Drop ids that have left the snapshot: the server confirmed + * the removal (or the message drained as steering / the queue cleared), so the + * optimistic hide is no longer needed. This keeps the set bounded — it never + * outlives the rows it tracks. Pure — returns a NEW set (callers assign it to + * the reactive `$state`). + */ +export function reconcileCancelledIds( + messages: readonly QueuedMessage[], + cancelledIds: ReadonlySet<string>, +): ReadonlySet<string> { + if (cancelledIds.size === 0) return EMPTY_STRING_SET; + const stillQueued = new Set<string>(); + for (const m of messages) { + if (cancelledIds.has(m.id)) stillQueued.add(m.id); + } + // Same set back → return the input identity so the component's `$state` setter + // sees no change (avoids a spurious reactive cycle). + if (stillQueued.size === cancelledIds.size) { + let same = true; + for (const id of cancelledIds) { + if (!stillQueued.has(id)) { + same = false; + break; + } + } + if (same) return cancelledIds; + } + return stillQueued; +} + +const EMPTY_STRING_SET: ReadonlySet<string> = new Set<string>(); diff --git a/src/features/surface-host/ui/MessageQueueList.svelte b/src/features/surface-host/ui/MessageQueueList.svelte index 554fa02..b260de2 100644 --- a/src/features/surface-host/ui/MessageQueueList.svelte +++ b/src/features/surface-host/ui/MessageQueueList.svelte @@ -1,21 +1,92 @@ <script lang="ts"> - import { parseMessageQueuePayload } from "../logic/message-queue"; + import { + parseMessageQueuePayload, + reconcileCancelledIds, + selectVisibleMessages, + } from "../logic/message-queue"; - let { payload }: { readonly payload: unknown } = $props(); + let { + payload, + onCancel, + }: { + readonly payload: unknown; + /** + * Cancel (remove) a single queued message by id (`chat.queue.cancel`). + * Required-but-nullable (not optional) so a parent can thread a + * `| undefined` callback through under `exactOptionalPropertyTypes`: + * `undefined` → a read-only list (no × affordance), e.g. a generic surface + * context with no conversation scope. The list still reconciles from the + * surface either way. + */ + readonly onCancel: ((messageId: string) => void) | undefined; + } = $props(); // Parse defensively; an unparseable payload yields null → render nothing // (graceful skip, per the custom-field contract). const data = $derived(parseMessageQueuePayload(payload)); + + // Optimistic-removal: a cancelled id is hidden the instant the user clicks, + // ahead of the surface's post-cancel snapshot. Reconcile on every payload + // change so a confirmed-removed id (no longer in the snapshot) is dropped + // from the set — keeping it bounded (pure helpers in logic/message-queue). + let cancelledIds = $state<ReadonlySet<string>>(new Set()); + + $effect(() => { + const parsed = data; + if (parsed === null) return; + const next = reconcileCancelledIds(parsed.messages, cancelledIds); + if (next !== cancelledIds) cancelledIds = next; + }); + + const visible = $derived( + data === null ? [] : selectVisibleMessages(data.messages, cancelledIds), + ); + + function handleCancel(messageId: string): void { + // Optimistically hide the row + fire the cancel (fire-and-forget; the + // surface update reconciles). Idempotent server-side, so a double-click or + // a cancel of an already-drained message is a silent no-op — no rollback. + if (cancelledIds.has(messageId)) return; + const next = new Set(cancelledIds); + next.add(messageId); + cancelledIds = next; + onCancel?.(messageId); + } </script> {#if data !== null && data.messages.length > 0} <ul class="flex flex-col gap-1 text-sm"> - {#each data.messages as msg (msg.id)} - <li class="rounded-box bg-base-200 px-3 py-2"> - <p class="whitespace-pre-wrap">{msg.text}</p> - <time class="text-xs opacity-50" datetime={new Date(msg.queuedAt).toISOString()}> - {new Date(msg.queuedAt).toLocaleTimeString()} - </time> + {#each visible as msg (msg.id)} + <li class="flex items-start gap-2 rounded-box bg-base-200 px-3 py-2"> + <div class="min-w-0 flex-1"> + <p class="whitespace-pre-wrap break-words">{msg.text}</p> + <time class="text-xs opacity-50" datetime={new Date(msg.queuedAt).toISOString()}> + {new Date(msg.queuedAt).toLocaleTimeString()} + </time> + </div> + {#if onCancel !== undefined} + <button + type="button" + class="btn btn-ghost btn-xs btn-square shrink-0 opacity-60 hover:opacity-100" + title="Cancel this queued message" + aria-label="Cancel this queued message" + onclick={() => handleCancel(msg.id)} + > + <svg + xmlns="http://www.w3.org/2000/svg" + viewBox="0 0 24 24" + fill="none" + stroke="currentColor" + stroke-width="2.5" + stroke-linecap="round" + stroke-linejoin="round" + class="h-3.5 w-3.5" + > + <line x1="18" y1="6" x2="6" y2="18"></line> + <line x1="6" y1="6" x2="18" y2="18"></line> + </svg> + </button> + {/if} </li> {/each} </ul> diff --git a/src/features/surface-host/ui/MessageQueueList.test.ts b/src/features/surface-host/ui/MessageQueueList.test.ts new file mode 100644 index 0000000..53044b3 --- /dev/null +++ b/src/features/surface-host/ui/MessageQueueList.test.ts @@ -0,0 +1,135 @@ +import type { QueuedMessage } from "@dispatch/wire"; +import { render, screen } from "@testing-library/svelte"; +import userEvent from "@testing-library/user-event"; +import { describe, expect, it, vi } from "vitest"; +import MessageQueueList from "./MessageQueueList.svelte"; + +function msg(id: string, text: string, queuedAt = 1_700_000_000_000): QueuedMessage { + return { id, text, queuedAt }; +} + +/** Build the message-queue surface field payload. */ +function payload(messages: readonly QueuedMessage[]): { messages: readonly QueuedMessage[] } { + return { messages }; +} + +describe("MessageQueueList", () => { + it("renders each queued message's text", () => { + render(MessageQueueList, { + props: { + payload: payload([msg("m1", "steer left"), msg("m2", "go right")]), + onCancel: undefined, + }, + }); + expect(screen.getByText("steer left")).toBeInTheDocument(); + expect(screen.getByText("go right")).toBeInTheDocument(); + }); + + it("renders nothing when the queue is empty", () => { + const { container } = render(MessageQueueList, { + props: { payload: payload([]), onCancel: undefined }, + }); + expect(container.querySelector("ul")).toBeNull(); + }); + + it("renders nothing for a malformed payload (graceful skip)", () => { + const { container } = render(MessageQueueList, { + props: { payload: { nope: true }, onCancel: undefined }, + }); + expect(container.querySelector("ul")).toBeNull(); + }); + + it("omits the cancel button when no onCancel is wired (read-only)", () => { + render(MessageQueueList, { + props: { payload: payload([msg("m1", "steer")]), onCancel: undefined }, + }); + expect(screen.queryByRole("button", { name: /cancel/i })).toBeNull(); + }); + + it("renders a cancel button per row when onCancel is wired", () => { + render(MessageQueueList, { + props: { payload: payload([msg("m1", "a"), msg("m2", "b")]), onCancel: vi.fn() }, + }); + expect(screen.getAllByRole("button", { name: /cancel/i })).toHaveLength(2); + }); + + it("clicking cancel fires onCancel with the row's message id and optimistically hides the row", async () => { + const user = userEvent.setup(); + const onCancel = vi.fn(); + render(MessageQueueList, { + props: { payload: payload([msg("m1", "keep me"), msg("m2", "cancel me")]), onCancel }, + }); + + // Both rows visible before click. + expect(screen.getByText("keep me")).toBeInTheDocument(); + expect(screen.getByText("cancel me")).toBeInTheDocument(); + + const buttons = screen.getAllByRole("button", { name: /cancel/i }); + // Cancel the SECOND row (m2). Buttons mirror row order. + const cancelM2 = buttons[1]; + if (cancelM2 === undefined) throw new Error("expected two cancel buttons"); + await user.click(cancelM2); + + expect(onCancel).toHaveBeenCalledTimes(1); + expect(onCancel).toHaveBeenCalledWith("m2"); + // m2 is optimistically hidden immediately; m1 remains. + expect(screen.getByText("keep me")).toBeInTheDocument(); + expect(screen.queryByText("cancel me")).toBeNull(); + }); + + it("does not fire onCancel twice for a double-click on the same row (idempotent client-side)", async () => { + const user = userEvent.setup(); + const onCancel = vi.fn(); + render(MessageQueueList, { + props: { payload: payload([msg("m1", "x")]), onCancel }, + }); + + const button = screen.getByRole("button", { name: /cancel/i }); + await user.click(button); + // The row is gone after the first click; the button left the DOM, so a + // second click on the stale element is a no-op — onCancel fires once. + await user.click(button).catch(() => {}); + expect(onCancel).toHaveBeenCalledTimes(1); + }); + + it("reconciles from the surface: a cancelled id gone from the snapshot clears the optimistic hide", async () => { + const user = userEvent.setup(); + const onCancel = vi.fn(); + const { rerender } = render(MessageQueueList, { + props: { payload: payload([msg("m1", "a"), msg("m2", "b")]), onCancel }, + }); + + // Cancel m1 — it hides optimistically. + const cancelButtons = screen.getAllByRole("button", { name: /cancel/i }); + const cancelM1 = cancelButtons[0]; + if (cancelM1 === undefined) throw new Error("expected a cancel button"); + await user.click(cancelM1); + expect(screen.queryByText("a")).toBeNull(); + expect(screen.getByText("b")).toBeInTheDocument(); + + // The surface pushes the post-cancel snapshot: m1 is gone (server confirmed). + // A NEW message m3 arrives in the same snapshot. The list renders m2 + m3. + rerender({ payload: payload([msg("m2", "b"), msg("m3", "c")]), onCancel }); + expect(screen.queryByText("a")).toBeNull(); + expect(screen.getByText("b")).toBeInTheDocument(); + expect(screen.getByText("c")).toBeInTheDocument(); + }); + + it("re-shows a row if the surface snapshot still contains a cancelled id (cancel not yet confirmed)", async () => { + // Edge case: the cancel is in flight and the surface hasn't updated yet, but + // a re-render with the SAME snapshot must keep the row hidden (optimistic). + const user = userEvent.setup(); + const onCancel = vi.fn(); + const samePayload = payload([msg("m1", "a")]); + const { rerender } = render(MessageQueueList, { + props: { payload: samePayload, onCancel }, + }); + + await user.click(screen.getByRole("button", { name: /cancel/i })); + expect(screen.queryByText("a")).toBeNull(); + + // Re-render with the same (stale) snapshot — the row stays hidden. + rerender({ payload: payload([msg("m1", "a")]), onCancel }); + expect(screen.queryByText("a")).toBeNull(); + }); +}); diff --git a/src/features/surface-host/ui/SurfaceView.svelte b/src/features/surface-host/ui/SurfaceView.svelte index aed8d03..8ce8ade 100644 --- a/src/features/surface-host/ui/SurfaceView.svelte +++ b/src/features/surface-host/ui/SurfaceView.svelte @@ -11,7 +11,22 @@ import TodoList from "./TodoList.svelte"; import Toggle from "./Toggle.svelte"; - let { spec, onInvoke }: { spec: SurfaceSpec; onInvoke: (msg: InvokeMessage) => void } = $props(); + let { + spec, + onInvoke, + onCancelQueuedMessage, + }: { + spec: SurfaceSpec; + onInvoke: (msg: InvokeMessage) => void; + /** + * Cancel a queued message by id — threaded ONLY to the `message-queue` + * renderer. Optional + scoped: generic surfaces (which pass nothing) keep + * rendering a read-only queue list. Kept as a typed callback (never a + * stringly-typed bus); the renderer dispatch is on `rendererId` (a renderer + * KIND), never the surface id. + */ + onCancelQueuedMessage?: (messageId: string) => void; + } = $props(); const plan = $derived(planSurface(spec)); // Consecutive stats render together as one aligned table; everything else is @@ -40,7 +55,7 @@ {#if group.field.rendererId === "table"} <SurfaceTable payload={group.field.payload} /> {:else if group.field.rendererId === "message-queue"} - <MessageQueueList payload={group.field.payload} /> + <MessageQueueList payload={group.field.payload} onCancel={onCancelQueuedMessage} /> {:else if group.field.rendererId === "todo"} <TodoList payload={group.field.payload} /> {/if} |