diff --git a/packages/chat-ui/src/composer.tsx b/packages/chat-ui/src/composer.tsx index f670ae349..2032ce8b7 100644 --- a/packages/chat-ui/src/composer.tsx +++ b/packages/chat-ui/src/composer.tsx @@ -369,6 +369,13 @@ export const Composer = forwardRef< const textareaRef = useRef(null); const fileInputRef = useRef(null); const attachGenerationRef = useRef(0); + // `sending` is React state, set only after `onSend` has already been + // awaited into — two calls to `performSend` inside one synchronous tick + // (an Enter keydown and a click both firing before a render lands) would + // both read `sending === false` and both post. This ref is set the + // instant a send starts, synchronously ahead of any render, so a second + // call in the same tick is turned away (CL-7198). + const sendInFlightRef = useRef(false); /** Auto-grow: the textarea reports its own content height, so the * measurement resets to the CSS-declared min-height before reading @@ -455,10 +462,16 @@ export const Composer = forwardRef< * recovering the text is the pending bubble's own Discard action. */ async function performSend(payload: ComposerSendPayload): Promise { + if (sendInFlightRef.current) return; + sendInFlightRef.current = true; setSending(true); setErrorMessage(null); - await onSend(payload); - setSending(false); + try { + await onSend(payload); + } finally { + sendInFlightRef.current = false; + setSending(false); + } } function runSlashCommand(command: SlashCommandSpec) { diff --git a/packages/chat-ui/src/use-optimistic-sends.ts b/packages/chat-ui/src/use-optimistic-sends.ts index 260438ef2..3b91bc2f0 100644 --- a/packages/chat-ui/src/use-optimistic-sends.ts +++ b/packages/chat-ui/src/use-optimistic-sends.ts @@ -12,7 +12,7 @@ // either arrival shows up with a matching `clientId`, so whichever wins // that race, the other is a no-op. -import { useEffect, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { useQueryClient } from "@tanstack/react-query"; import { ChatApiError, pingWorkbenchPresence, sendMessage } from "./api"; import type { MessageItem, MessagesResponse } from "./api"; @@ -172,6 +172,16 @@ export function useOptimisticSends(args: { setPendingSends([]); }, [activeWorkbenchId]); + // `sendPending` closes over the `activeWorkbenchId` its own render was + // called with, which stays fixed for the life of that async call. This + // ref tracks the live value so a continuation resolving after the reader + // has switched benches can tell its own send is no longer for the + // workbench currently on screen (CL-7198). + const activeWorkbenchIdRef = useRef(activeWorkbenchId); + useEffect(() => { + activeWorkbenchIdRef.current = activeWorkbenchId; + }, [activeWorkbenchId]); + async function sendPending( nonce: string, text: string, @@ -290,7 +300,14 @@ export function useOptimisticSends(args: { }); }, ); - if (pendingParentMessageId !== null) { + // A continuation that started before the reader switched to a + // different workbench must not drag them back into this one's + // thread — `useThreadNavigation` already reset the open thread on + // that switch; re-opening it here would undo that reset (CL-7198). + if ( + pendingParentMessageId !== null && + activeWorkbenchIdRef.current === activeWorkbenchId + ) { openThreadById(threadId); } } diff --git a/packages/chat-ui/src/use-workbench-feed.ts b/packages/chat-ui/src/use-workbench-feed.ts index 06dcbb9ea..16fbf6e27 100644 --- a/packages/chat-ui/src/use-workbench-feed.ts +++ b/packages/chat-ui/src/use-workbench-feed.ts @@ -501,9 +501,19 @@ export function useWorkbenchFeed(args: { }): WorkbenchFeed { const { tenantId, activeWorkbenchId, onWorkbenchNotFound } = args; const queryClient = useQueryClient(); - const refreshTimerRef = useRef | undefined>( - undefined, - ); + // Scoped to the (tenantId, workbenchId) the timer was scheduled for — + // not a bare timer handle — so a pending refresh for one workbench can + // never make another workbench's `refreshFeed()` call early-return, and + // never fires an invalidation against a query key that is no longer the + // active one (CL-7198). + const refreshTimerRef = useRef< + | { + readonly tenantId: string; + readonly workbenchId: string; + readonly timer: ReturnType; + } + | undefined + >(undefined); // Threads and the mailbox are two queries, and every view the timeline // can show is derived from them (CL-6313). Each message carries the @@ -600,22 +610,39 @@ export function useWorkbenchFeed(args: { // completes, so invalidating on each one still walks the hub once per // gap between responses. Collapsing the burst into one refetch per // window is the difference between ~40 requests per turn and ~2. - if (refreshTimerRef.current !== undefined) return; - refreshTimerRef.current = setTimeout(() => { - refreshTimerRef.current = undefined; - void queryClient.invalidateQueries({ - queryKey: chatFeedQueryKeyPrefix(tenantId, activeWorkbenchId), - }); - }, CHAT_FEED_COALESCE_MS); + const pending = refreshTimerRef.current; + if (pending !== undefined) { + if ( + pending.tenantId === tenantId && + pending.workbenchId === activeWorkbenchId + ) { + return; + } + // A timer scheduled for a workbench the reader has since left — + // clear it rather than let it invalidate that now-inactive query key. + clearTimeout(pending.timer); + } + const workbenchId = activeWorkbenchId; + refreshTimerRef.current = { + tenantId, + workbenchId, + timer: setTimeout(() => { + refreshTimerRef.current = undefined; + void queryClient.invalidateQueries({ + queryKey: chatFeedQueryKeyPrefix(tenantId, workbenchId), + }); + }, CHAT_FEED_COALESCE_MS), + }; }, [queryClient, tenantId, activeWorkbenchId, isUnauthorized]); useEffect( () => () => { if (refreshTimerRef.current !== undefined) { - clearTimeout(refreshTimerRef.current); + clearTimeout(refreshTimerRef.current.timer); + refreshTimerRef.current = undefined; } }, - [], + [tenantId, activeWorkbenchId], ); return { threads, diff --git a/packages/chat-ui/test/composer-synchronous-double-send.test.tsx b/packages/chat-ui/test/composer-synchronous-double-send.test.tsx new file mode 100644 index 000000000..49873bef0 --- /dev/null +++ b/packages/chat-ui/test/composer-synchronous-double-send.test.tsx @@ -0,0 +1,104 @@ +// CL-7198: the composer's send guard used to test the `sending` *state* +// variable, which `performSend` only sets after the click/keydown handler +// that started it has already returned. Two triggers landing in the same +// synchronous tick (e.g. a stray double dispatch of the send action) both +// read `sending === false` and both post — this proves a second trigger in +// the same tick is turned away regardless of state timing. + +import { afterEach, describe, expect, test } from "bun:test"; +import { act, createElement, createRef } from "react"; +import { createRoot } from "react-dom/client"; +import type { Root } from "react-dom/client"; + +import { Composer } from "../src/composer"; +import type { ComposerHandle, ComposerSendPayload } from "../src/composer"; + +let container: HTMLDivElement | null = null; +let root: Root | null = null; + +afterEach(() => { + if (root !== null) { + act(() => root?.unmount()); + root = null; + } + if (container !== null) { + container.remove(); + container = null; + } +}); + +const settle = () => + act(() => new Promise((resolve) => setTimeout(resolve, 0))); + +function mount(onSend: (payload: ComposerSendPayload) => Promise) { + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + const ref = createRef(); + act(() => { + root?.render( + createElement(Composer, { + ref, + agents: [], + onSend, + onInviteAgent: () => undefined, + onOpenAgentsSettings: () => undefined, + onCreateRoutineInSpace: () => undefined, + }), + ); + }); + return container; +} + +function textarea(): HTMLTextAreaElement { + const element = container?.querySelector("textarea"); + if (element === null || element === undefined) { + throw new Error("composer textarea not found"); + } + return element; +} + +function typeInto(element: HTMLTextAreaElement, text: string) { + const setter = Object.getOwnPropertyDescriptor( + globalThis.HTMLTextAreaElement.prototype, + "value", + )?.set; + act(() => { + setter?.call(element, text); + element.dispatchEvent(new Event("input", { bubbles: true })); + }); +} + +function sendButton(): HTMLButtonElement { + const button = container?.querySelector( + '[aria-label^="Send"]', + ); + if (button === null || button === undefined) { + throw new Error("send button not found"); + } + return button; +} + +describe("Composer synchronous double-send guard (CL-7198)", () => { + test("two clicks in the same tick post exactly one send", async () => { + let sendCount = 0; + const payloads: ComposerSendPayload[] = []; + mount((payload) => { + sendCount += 1; + payloads.push(payload); + return Promise.resolve(true); + }); + + typeInto(textarea(), "hello there"); + await settle(); + + act(() => { + sendButton().click(); + sendButton().click(); + }); + await settle(); + + expect(sendCount).toBe(1); + expect(payloads).toEqual([{ text: "hello there", attachments: [] }]); + }); +}); diff --git a/packages/chat-ui/test/use-optimistic-sends-stale-navigation.test.tsx b/packages/chat-ui/test/use-optimistic-sends-stale-navigation.test.tsx new file mode 100644 index 000000000..94598f50e --- /dev/null +++ b/packages/chat-ui/test/use-optimistic-sends-stale-navigation.test.tsx @@ -0,0 +1,187 @@ +// CL-7198: a send continuation captures the workbench id at send time and +// used to re-open its freshly-created reply thread even after the reader +// had already switched to a different workbench — see +// `use-optimistic-sends.ts`'s `openThreadById` call. This mounts the real +// hook against a DOM (see dom-setup.ts) and a stubbed `fetch` (never +// `mock.module`, per test/chat-workspace.test.tsx's own note) so the +// continuation's timing is real, not simulated. + +import { afterEach, describe, expect, test } from "bun:test"; +import { act, createElement, useState } from "react"; +import { createRoot } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +import { useOptimisticSends } from "../src/use-optimistic-sends"; +import type { ComposerSendPayload } from "../src/composer"; + +const realFetch = globalThis.fetch; + +afterEach(() => { + globalThis.fetch = realFetch; +}); + +type PendingResponse = { + readonly resolve: (body: unknown) => void; +}; + +function stubFetch(): { readonly nextSend: () => PendingResponse } { + const queue: ((body: unknown) => void)[] = []; + globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const path = typeof input === "string" ? input : String(input); + if (init?.method === "POST" && /\/messages$/.test(path)) { + return new Promise((resolve) => { + queue.push((body: unknown) => { + resolve( + new Response(JSON.stringify(body), { + status: 200, + headers: { "content-type": "application/json" }, + }), + ); + }); + }); + } + if (init?.method === "POST" && /\/presence$/.test(path)) { + return new Response(null, { status: 204 }); + } + throw new Error(`unstubbed fetch: ${path}`); + }) as typeof fetch; + return { + nextSend: () => { + const resolve = queue.shift(); + if (resolve === undefined) throw new Error("no send in flight"); + return { resolve }; + }, + }; +} + +function mount(initialWorkbenchId: string) { + const container = document.createElement("div"); + document.body.appendChild(container); + const queryClient = new QueryClient(); + const root = createRoot(container); + const openThreadCalls: string[] = []; + let setWorkbenchId: (id: string) => void = () => undefined; + let handleSend: (payload: ComposerSendPayload) => Promise = () => + Promise.resolve(false); + + function Host() { + const [workbenchId, updateWorkbenchId] = useState(initialWorkbenchId); + setWorkbenchId = updateWorkbenchId; + const optimistic = useOptimisticSends({ + tenantId: "ten_1", + activeWorkbenchId: workbenchId, + currentUserPrincipalId: "prn_self", + openThreadId: null, + pendingParentMessageId: "msg_parent", + openThreadById: (threadId: string) => { + openThreadCalls.push(threadId); + }, + noteAwaitingReply: () => undefined, + hasAgentParticipant: false, + restoreDraft: () => undefined, + }); + handleSend = optimistic.handleSend; + return null; + } + + act(() => { + root.render( + createElement( + QueryClientProvider, + { client: queryClient }, + createElement(Host), + ), + ); + }); + + return { + send: (payload: ComposerSendPayload) => { + let result: Promise = Promise.resolve(false); + act(() => { + result = handleSend(payload); + }); + return result; + }, + switchWorkbench: (id: string) => + act(() => { + setWorkbenchId(id); + }), + openThreadCalls: () => openThreadCalls, + settle: () => act(() => Promise.resolve().then(() => Promise.resolve())), + unmount: () => act(() => root.unmount()), + }; +} + +describe("useOptimisticSends — stale thread navigation (CL-7198)", () => { + test("a send continuation no-ops its navigation once the reader has switched to a different workbench", async () => { + const { nextSend } = stubFetch(); + const harness = mount("ch_a"); + + const sendPromise = harness.send({ text: "hello", attachments: [] }); + const inFlight = nextSend(); + + harness.switchWorkbench("ch_b"); + + await act(async () => { + inFlight.resolve({ + id: "msg_1", + createdAt: "2026-01-01T00:00:00.000Z", + threadId: "thr_1", + clientId: "pending_1", + }); + await sendPromise; + }); + + expect(harness.openThreadCalls()).toEqual([]); + harness.unmount(); + }); + + test("a send continuation still navigates when the reader is on the same workbench once it resolves", async () => { + const { nextSend } = stubFetch(); + const harness = mount("ch_a"); + + const sendPromise = harness.send({ text: "hello", attachments: [] }); + const inFlight = nextSend(); + + await act(async () => { + inFlight.resolve({ + id: "msg_1", + createdAt: "2026-01-01T00:00:00.000Z", + threadId: "thr_1", + clientId: "pending_1", + }); + await sendPromise; + }); + + expect(harness.openThreadCalls()).toEqual(["thr_1"]); + harness.unmount(); + }); + + test("switching away and back to the same workbench before the send resolves still navigates", async () => { + const { nextSend } = stubFetch(); + const harness = mount("ch_a"); + + const sendPromise = harness.send({ text: "hello", attachments: [] }); + const inFlight = nextSend(); + + harness.switchWorkbench("ch_b"); + harness.switchWorkbench("ch_a"); + + await act(async () => { + inFlight.resolve({ + id: "msg_1", + createdAt: "2026-01-01T00:00:00.000Z", + threadId: "thr_1", + clientId: "pending_1", + }); + await sendPromise; + }); + + // The reader is back on the workbench the send targeted, so the thread + // it created is exactly what they'd expect to see — this is a + // deliberate outcome of comparing against the *current* workbench, + // not stale detection of "did anything change in between". + expect(harness.openThreadCalls()).toEqual(["thr_1"]); + harness.unmount(); + }); +}); diff --git a/packages/chat-ui/test/use-workbench-feed-refresh-scoping.test.tsx b/packages/chat-ui/test/use-workbench-feed-refresh-scoping.test.tsx new file mode 100644 index 000000000..96cb30919 --- /dev/null +++ b/packages/chat-ui/test/use-workbench-feed-refresh-scoping.test.tsx @@ -0,0 +1,127 @@ +// CL-7198: the coalescing refresh timer was a single ref, neither scoped +// to nor cleared on the active workbench id. A pending timer scheduled +// for one workbench made a different workbench's `refreshFeed()` call +// early-return, and the timer then invalidated the first workbench's own +// (now-inactive) query key instead. Stubs `globalThis.fetch` — never +// `mock.module` for `../src/api`, per test/chat-workspace.test.tsx's note. + +import { afterEach, describe, expect, test } from "bun:test"; +import { act, createElement, useState } from "react"; +import { createRoot } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; + +import { + chatFeedQueryKeyPrefix, + useWorkbenchFeed, +} from "../src/use-workbench-feed"; + +const realFetch = globalThis.fetch; + +afterEach(() => { + globalThis.fetch = realFetch; +}); + +function stubFetch() { + globalThis.fetch = (async (input: RequestInfo | URL) => { + const path = typeof input === "string" ? input : String(input); + const json = (body: unknown) => + new Response(JSON.stringify(body), { + status: 200, + headers: { "content-type": "application/json" }, + }); + if (/\/threads$/.test(path)) return json({ rootThreadId: "", items: [] }); + if (/\/messages/.test(path)) return json({ items: [] }); + if (/\/pins$/.test(path)) return json({ items: [] }); + throw new Error(`unstubbed fetch: ${path}`); + }) as typeof fetch; +} + +const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +function mount(tenantId: string, initialWorkbenchId: string) { + const container = document.createElement("div"); + document.body.appendChild(container); + const queryClient = new QueryClient(); + const invalidateCalls: (readonly unknown[])[] = []; + const originalInvalidate = queryClient.invalidateQueries.bind(queryClient); + queryClient.invalidateQueries = (( + filters?: Parameters[0], + ) => { + if (filters?.queryKey !== undefined) { + invalidateCalls.push(filters.queryKey); + } + return originalInvalidate(filters); + }) as typeof queryClient.invalidateQueries; + + const root = createRoot(container); + let setWorkbenchId: (id: string) => void = () => undefined; + let refreshFeed: () => void = () => undefined; + + function Host() { + const [workbenchId, updateWorkbenchId] = useState(initialWorkbenchId); + setWorkbenchId = updateWorkbenchId; + const feed = useWorkbenchFeed({ tenantId, activeWorkbenchId: workbenchId }); + refreshFeed = feed.refreshFeed; + return null; + } + + act(() => { + root.render( + createElement( + QueryClientProvider, + { client: queryClient }, + createElement(Host), + ), + ); + }); + + return { + switchWorkbench: (id: string) => + act(() => { + setWorkbenchId(id); + }), + refreshFeed: () => + act(() => { + refreshFeed(); + }), + settle: (ms: number) => act(() => sleep(ms)), + invalidateCalls: () => invalidateCalls, + unmount: () => act(() => root.unmount()), + }; +} + +describe("useWorkbenchFeed — refresh timer scoping across a workbench switch (CL-7198)", () => { + test("a refresh scheduled for bench A never fires, and bench B's own refresh fires once switching happens before the coalesce window closes", async () => { + stubFetch(); + const harness = mount("ten_1", "ch_a"); + await harness.settle(0); + + harness.refreshFeed(); + harness.switchWorkbench("ch_b"); + harness.refreshFeed(); + + await harness.settle(400); + + expect(harness.invalidateCalls()).toEqual([ + chatFeedQueryKeyPrefix("ten_1", "ch_b"), + ]); + harness.unmount(); + }); + + test("a second refresh call for the same bench inside the coalesce window is a no-op, not a second timer", async () => { + stubFetch(); + const harness = mount("ten_1", "ch_a"); + await harness.settle(0); + + harness.refreshFeed(); + harness.refreshFeed(); + harness.refreshFeed(); + + await harness.settle(400); + + expect(harness.invalidateCalls()).toEqual([ + chatFeedQueryKeyPrefix("ten_1", "ch_a"), + ]); + harness.unmount(); + }); +});