From 3fd2a33daea511435b71d405cbde40af6fe39ba0 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 29 Aug 2026 23:55:50 -0700 Subject: [PATCH 1/6] Add tests for stale thread navigation after a workbench switch A send continuation reopens the reply thread it just created using the workbench id it was sent with, with no check that workbench is still the one on screen. Covers a continuation resolving after a switch away (no navigation), after resolving on the same bench (navigates as before), and after switching away and back (still navigates, since the reader is where the send targeted). --- ...optimistic-sends-stale-navigation.test.tsx | 187 ++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 packages/chat-ui/test/use-optimistic-sends-stale-navigation.test.tsx 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(); + }); +}); From d4ccefd67d89beddf110685425d0e4e9c6c55560 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 29 Aug 2026 23:56:13 -0700 Subject: [PATCH 2/6] No-op a send continuation's thread navigation on a stale workbench MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sendPending closes over the activeWorkbenchId its own render was called with, which never changes for the life of that async call. If the reader switches workbenches while a reply's first send is still in flight, the continuation resolved and reopened that reply thread regardless — dragging the reader back into a bench they had already left, undoing use-thread-navigation's own reset-on-switch effect. A ref now tracks the live activeWorkbenchId; the navigation call only fires when it still matches the value the send was made with. --- packages/chat-ui/src/use-optimistic-sends.ts | 21 ++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) 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); } } From ee7b82bd0af3bddcb97162a5793c7aa508dcc901 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 29 Aug 2026 23:56:33 -0700 Subject: [PATCH 3/6] Add a test for the composer's synchronous double-send guard The send guard tested the sending state variable, which performSend only sets after the caller that started it has already returned. Two triggers landing in the same tick both read sending === false and both post. Proves a second click in the same tick as the first is turned away. --- .../composer-synchronous-double-send.test.tsx | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 packages/chat-ui/test/composer-synchronous-double-send.test.tsx 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: [] }]); + }); +}); From 01a08f7ca226f569e39f258bd903c9e5c7a752ea Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 29 Aug 2026 23:56:58 -0700 Subject: [PATCH 4/6] Guard the composer's send with a synchronous in-flight ref canSendComposerAction's sending check reads React state, which performSend only sets true after the event handler that called it has already returned to the browser. Two sends triggered in the same synchronous tick both read sending === false and both post, producing two requests with two distinct nonces that nothing downstream dedupes. The guard now lives in performSend itself (covering every caller, including the /summarize slash command) and flips synchronously before any await, so a second call in the same tick is turned away. --- packages/chat-ui/src/composer.tsx | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) 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) { From 7fc88efdb1e033043e99a89c0d892ff4d152e382 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 29 Aug 2026 23:57:18 -0700 Subject: [PATCH 5/6] Add tests for the feed refresh timer's per-workbench scoping The coalescing refresh timer was a single ref, neither scoped to nor cleared on the active workbench. A pending timer scheduled for one workbench made a different workbench's own refreshFeed() call early-return, and the timer then invalidated the first workbench's now- inactive query key. Proves a refresh scheduled just before a switch never fires for the old bench and the new bench's own refresh fires instead, and that repeat calls for the same bench still coalesce into one. --- ...se-workbench-feed-refresh-scoping.test.tsx | 127 ++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100644 packages/chat-ui/test/use-workbench-feed-refresh-scoping.test.tsx 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(); + }); +}); From 7d031c9ea77b136bb2f18762004906093af937be Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 29 Aug 2026 23:57:48 -0700 Subject: [PATCH 6/6] Scope the feed refresh timer to its own workbench and clear it on switch refreshTimerRef held a bare timer handle with no record of which workbench scheduled it. Switching benches while a refresh was pending made the new bench's own refreshFeed() call early-return on the old timer, and when that timer fired it invalidated the old (now-inactive) query key instead of ever refreshing the bench actually on screen. The ref now carries the (tenantId, workbenchId) it was scheduled for; refreshFeed clears a timer scheduled for a different one instead of deferring to it, and the unmount effect's dependency array was widened so the same clear runs on every switch, not only on unmount. --- packages/chat-ui/src/use-workbench-feed.ts | 51 +++++++++++++++++----- 1 file changed, 39 insertions(+), 12 deletions(-) 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,