From c3beae1e180f16c3d821c22e98deee3efc31ef69 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 1 Sep 2026 11:15:42 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=8C=20refactor:=20Give=20the=20Subagen?= =?UTF-8?q?t=20Surface=20a=20Host=20Seam=20and=20Its=20Own=20Jotai=20State?= =?UTF-8?q?=20(#15455)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ๐Ÿ“ docs: Record Who Owns Which Piece of Client State * ๐Ÿ”Œ refactor: Give the Subagent Surface a Host Seam and Its Own Jotai State The subagent chat surface reached up into seven application atoms and could not be lifted anywhere, and the six atoms it owns were still Recoil while its sibling stores had already moved to Jotai. Feature-owned state converts to Jotai and stays where it is: the selection, per-task control receipts, live progress and the parent-stream fence. The control atom keeps its session-storage retry โ€” restored per store rather than per family member, so a reload still sees what the last one persisted. App-global state now arrives through one `ChatSurface` value the host supplies: `enterToSend`, `maximizeChatSpace`, `showScrollButton`, the reader's composer bindings, a `claimForeground` command for the slot the panel and the artifacts panel share, and `handOffComposerText` for a continuation's unsent words. Two hosts mount the same adapter โ€” the conversation view and the shared-link view. Opening the panel was three copies of "clear the artifacts slot, then select"; that is now `useOpenSubagentPanel`, and the three call sites say what they mean. Nothing under `Chat/Subagents/**` imports `~/store` or `recoil` any more, and its whole suite runs with no `RecoilRoot` at all. * ๐Ÿงน refactor: Move Subagent State Into the Feature and Free Its Drained Atoms Two review findings, both fair. The selection atom still lived in `~/store`, which handed the supposedly liftable surface an implicit dependency on the application store alongside its declared host interface โ€” and contradicted the ownership rule this branch added to CLAUDE.md two commits ago. The module moves to `components/Chat/Subagents/state.ts`; the hosts that write it (the SSE step handler, the message renderers) now import from the feature, which is the direction that survives an extraction. The conversation-switch drain blanked its two atom-family members but left them cached. Every invocation key is unique, so a long-lived tab accumulated two atom configurations per subagent call it ever saw โ€” exactly what the comment at that boundary says it exists to prevent. `removeSubagentProgressAtoms` releases them after the values are cleared, mirroring `removeUsageAtoms`. The new test fails without the release. * ๐Ÿ”Ž fix: Let a Subagent Card Render Where There Is No Panel to Open Search results render the same card through `Part`, and that route is not beneath either chat surface โ€” so a search hit containing a subagent tool call threw on `useChatSurface()` before it could render at all. Rendering the card and opening a panel are different requirements. The host stays required for the panel, the activity view and the event group, all of which only ever mount inside one. `useOpenSubagentPanel` reads the context optionally instead and answers `null` where there is no host, which is also the honest answer for search: there is no panel on that route to open, so the card renders with its details control disabled. The new test renders the card with no surface at all, and throws with the reported error when the hook is put back the way it was. * ๐Ÿงฎ fix: Enrol Read-Created Progress Atoms in the Drain Registry The read is what creates a family member, and only the streaming paths were registering their keys. A card rendering a conversation that finished streaming long ago โ€” or a search result that never streams at all โ€” created one the navigation drain could not see, so browsing history accumulated an atom configuration per card for the life of the tab. `useSubagentProgress` does both halves: it reads the invocation's progress and enrols its key, so every route into the family shares the one cleanup boundary. The panel and the parent card both read through it now. * ๐Ÿงฝ fix: Free a Read-Created Progress Atom at Its Own Card's Unmount Enrolling read-created keys in the drain registry only helps where something runs the drain, and `/search` is a standalone route that never mounts the chat event handlers. Scrolling a virtualised result list therefore kept minting members nothing would ever free. `useSubagentProgress` cleans up after itself instead: on unmount it frees the family members when the read is the only reason they exist. Two guards keep that from taking live history with it โ€” a key the stream registered belongs to the drain, and a member holding folded activity is the record of what the child did, so reopening its card still shows it. * ๐Ÿ”— fix: Free a Progress Atom Only After Its Last Reader Lets Go Opening a foreground panel before the first event gives one still-empty member two readers: the card that spawned the child and the panel showing it. Closing the panel freed it while the card was still subscribed โ€” and a subscriber does not re-render just because its atom left the family, so the first SSE update landed on the replacement member and the card's ticker stayed dead. `useSubagentProgress` counts its mounted readers and frees only when the last one lets go, alongside the two claims that already blocked it: a key the stream registered, and a member holding folded activity. --- AGENTS.md | 11 + CLAUDE.md | 16 + .../Messages/Content/Parts/SubagentCall.tsx | 42 +- .../Parts/__tests__/SubagentCall.test.tsx | 169 ++++--- .../Chat/Messages/Content/Wakeup.tsx | 20 +- .../Content/__tests__/Wakeup.test.tsx | 106 +++-- .../src/components/Chat/Presentation.test.tsx | 44 +- client/src/components/Chat/Presentation.tsx | 35 +- .../EventSubagentActivityGroup.test.tsx | 49 +- .../Subagents/EventSubagentActivityGroup.tsx | 28 +- .../SharedSubagentActivityDialog.test.tsx | 6 +- .../SharedSubagentActivityDialog.tsx | 8 +- .../Chat/Subagents/SubagentActivity.test.tsx | 7 +- .../Chat/Subagents/SubagentActivity.tsx | 5 +- .../Subagents/SubagentConversation.test.tsx | 14 +- .../Chat/Subagents/SubagentConversation.tsx | 5 +- .../Subagents/SubagentThreadPanel.test.tsx | 436 +++++++++--------- .../Chat/Subagents/SubagentThreadPanel.tsx | 60 +-- .../src/components/Chat/Subagents/adapters.ts | 4 +- .../Chat/Subagents/eventSelection.ts | 2 +- .../Chat/Subagents/state.spec.ts} | 99 +++- .../Chat/Subagents/state.ts} | 164 +++++-- .../src/components/Chat/Subagents/surface.tsx | 74 +++ client/src/components/Chat/Surface.tsx | 53 +++ .../Chat/__tests__/Surface.test.tsx | 84 ++++ client/src/components/Share/ShareView.tsx | 15 +- .../useSubagentActivityStream.test.tsx | 19 +- .../Subagents/useSubagentActivityStream.ts | 12 +- .../SSE/__tests__/useStepHandler.spec.ts | 80 ++-- client/src/hooks/SSE/useStepHandler.ts | 117 +++-- client/src/store/index.ts | 1 - client/test/harness.tsx | 59 +++ 32 files changed, 1189 insertions(+), 655 deletions(-) rename client/src/{store/subagents.spec.ts => components/Chat/Subagents/state.spec.ts} (77%) rename client/src/{store/subagents.ts => components/Chat/Subagents/state.ts} (80%) create mode 100644 client/src/components/Chat/Subagents/surface.tsx create mode 100644 client/src/components/Chat/Surface.tsx create mode 100644 client/src/components/Chat/__tests__/Surface.test.tsx create mode 100644 client/test/harness.tsx diff --git a/AGENTS.md b/AGENTS.md index 13a1570a26..78e72785f8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,3 +33,14 @@ why any new custom CSS cannot be expressed by the shared system. See the detaile When adding or changing code that mutates user documents, invalidate the auth user document cache for affected users, including bulk role and user mutations. See the detailed policy in `CLAUDE.md` under โ€œAuth cache invalidationโ€. + +## Client state ownership + +The client is migrating from Recoil to Jotai โ€” convert the areas you touch, not the whole store. +Split by ownership: state a feature both writes and reads is feature-owned, so convert it to Jotai +and keep it inside the feature; app-global preferences and shell state a feature merely consumes +(`maximizeChatSpace`, `showScrollButton`, `enterToSend`, artifact visibility) must be passed in +through props or a small host-supplied context rather than reached for through `~/store`. Passing +them in is what lets a feature move to its own workspace later without a rewrite, and it keeps the +Jotai conversion scoped to the state a feature owns. See the detailed policy in `CLAUDE.md` under +โ€œClient State Ownershipโ€. diff --git a/CLAUDE.md b/CLAUDE.md index 7594b29027..640ae92297 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -178,6 +178,22 @@ Multi-line imports count total character length across all lines. Consolidate va - React Query (`@tanstack/react-query`) for all API interactions; proper query invalidation on mutations. - QueryKeys and MutationKeys in `packages/data-provider/src/keys.ts`. +### Client State Ownership + +The client is migrating from Recoil to Jotai. Convert the areas you touch rather than +migrating wholesale, and split the work by who owns the state: + +- **Feature-owned state** โ€” atoms a single feature both writes and reads. Convert these to + Jotai as you touch them, and keep them inside the feature. +- **App-global state** โ€” preferences and shell state a feature merely consumes + (`maximizeChatSpace`, `showScrollButton`, `enterToSend`, artifact visibility). A feature + that could plausibly be extracted must not reach into `~/store` for these; accept them + through props or a small context the host supplies. + +Passing app-global state in โ€” rather than reaching for it โ€” is what lets a feature move to +its own workspace later without a rewrite, and it keeps the Jotai conversion scoped to the +state a feature actually owns instead of dragging the global migration forward early. + ### Data-Provider Integration - Endpoints: `packages/data-provider/src/api-endpoints.ts` diff --git a/client/src/components/Chat/Messages/Content/Parts/SubagentCall.tsx b/client/src/components/Chat/Messages/Content/Parts/SubagentCall.tsx index be4588632c..c87cb854d3 100644 --- a/client/src/components/Chat/Messages/Content/Parts/SubagentCall.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/SubagentCall.tsx @@ -1,7 +1,7 @@ import { useCallback, useContext, useEffect, useMemo, useReducer, useRef } from 'react'; +import { useSetAtom } from 'jotai'; import { ChevronRight, Users } from 'lucide-react'; import { EModelEndpoint } from 'librechat-data-provider'; -import { useRecoilValue, useResetRecoilState, useSetRecoilState } from 'recoil'; import type { PartMetadata, TAttachment, @@ -9,12 +9,13 @@ import type { TMessageContentParts, } from 'librechat-data-provider'; import type { SubagentTickerLine } from '~/utils/subagentContent'; -import store, { +import { activeSubagentPanel, - subagentProgressByToolCallId, subagentProgressKey, -} from '~/store'; + useSubagentProgress, +} from '~/components/Chat/Subagents/state'; import { adaptLivePersistedActivity } from '~/components/Chat/Subagents/adapters'; +import { useOpenSubagentPanel } from '~/components/Chat/Subagents/surface'; import { MessageContext } from '~/Providers/MessageContext'; import { useShareContext } from '~/Providers/ShareContext'; import MessageIcon from '~/components/Share/MessageIcon'; @@ -40,7 +41,7 @@ interface SubagentCallProps { output?: string | null; attachments?: TAttachment[]; /** Aggregated content parts the backend attached to the tool_call at - * message-save time. Takes precedence over the in-memory Recoil atom + * message-save time. Takes precedence over the in-memory atom * so a page refresh shows the same history the user saw live. Older * runs recorded before the persistence path landed will not have this * field; those fall back to the atom (or the raw `output` string). */ @@ -151,7 +152,7 @@ function useThrottledValue(value: T, intervalMs: number, enabled: boolean): T * artifacts-style panel that renders the child's aggregated activity through * the shared child-activity module, so every subagent mode uses one deep view. * - * Progress is sourced from the `subagentProgressByToolCallId` Recoil atom + * Progress is sourced from the `subagentProgressByToolCallId` atom * family, populated by `useStepHandler` as `ON_SUBAGENT_UPDATE` SSE * envelopes arrive. The atom is keyed by the parent message and * `tool_call_id`, since providers may reuse tool IDs across turns. @@ -172,12 +173,9 @@ export default function SubagentCall({ const parentMessageContext = useContext(MessageContext); const parentMessageId = parentMessageContext.messageId?.trim() ?? ''; const partIndex = parentMessageContext.partIndex ?? 0; - const progress = useRecoilValue( - subagentProgressByToolCallId(subagentProgressKey(parentMessageId, toolCallId, partIndex)), - ); - const setSelectedSubagent = useSetRecoilState(activeSubagentPanel); - const setArtifactsVisible = useSetRecoilState(store.artifactsVisibility); - const resetCurrentArtifactId = useResetRecoilState(store.currentArtifactId); + const progress = useSubagentProgress(subagentProgressKey(parentMessageId, toolCallId, partIndex)); + const setSelectedSubagent = useSetAtom(activeSubagentPanel); + const openPanel = useOpenSubagentPanel(); const agentsMap = useAgentsMapContext(); const backgroundHandle = useMemo( () => parseSubagentBackgroundHandle(output, args), @@ -301,10 +299,14 @@ export default function SubagentCall({ const canOpenLiveForeground = isSharedConvo !== true && backgroundHandle == null && hasParentContext; return ( - hasParentContext && (canOpenDurablePanel || canOpenLiveForeground || hasRenderableFallback) + /** No host, no panel to open โ€” a search result renders this same card. */ + openPanel != null && + hasParentContext && + (canOpenDurablePanel || canOpenLiveForeground || hasRenderableFallback) ); }, [ backgroundHandle, + openPanel, canOpenDurablePanel, initialProgress, isSharedConvo, @@ -369,17 +371,9 @@ export default function SubagentCall({ }, [panelSelection, parentMessageId, partIndex, setSelectedSubagent, toolCallId]); const openDetails = useCallback(() => { - if (!canOpenDetails) return; - resetCurrentArtifactId(); - setArtifactsVisible(false); - setSelectedSubagent(panelSelection); - }, [ - canOpenDetails, - panelSelection, - resetCurrentArtifactId, - setArtifactsVisible, - setSelectedSubagent, - ]); + if (!canOpenDetails || openPanel == null) return; + openPanel(panelSelection); + }, [canOpenDetails, openPanel, panelSelection]); return ( <> diff --git a/client/src/components/Chat/Messages/Content/Parts/__tests__/SubagentCall.test.tsx b/client/src/components/Chat/Messages/Content/Parts/__tests__/SubagentCall.test.tsx index 5d9ae75054..80e6da7654 100644 --- a/client/src/components/Chat/Messages/Content/Parts/__tests__/SubagentCall.test.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/__tests__/SubagentCall.test.tsx @@ -1,6 +1,7 @@ import React from 'react'; +import { RecoilRoot } from 'recoil'; +import { useAtomValue, useStore } from 'jotai'; import { MemoryRouter } from 'react-router-dom'; -import { RecoilRoot, useRecoilCallback, useRecoilValue } from 'recoil'; import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react'; import type { SubagentUpdateEvent } from 'librechat-data-provider'; import type { @@ -8,7 +9,7 @@ import type { SubagentContentPart, SubagentTickerState, } from '~/utils/subagentContent'; -import type { ActiveSubagentPanel, SubagentProgress } from '~/store/subagents'; +import type { ActiveSubagentPanel, SubagentProgress } from '~/components/Chat/Subagents/state'; import { foldSubagentEvent, foldSubagentEventIntoTicker, @@ -19,9 +20,10 @@ import { activeSubagentPanel, subagentProgressByToolCallId, subagentProgressKey, -} from '~/store/subagents'; +} from '~/components/Chat/Subagents/state'; import SubagentCall, { SUBAGENT_TICKER_THROTTLE_MS } from '../SubagentCall'; import { MessageContext } from '~/Providers/MessageContext'; +import { ChatSurfaceHarness } from 'test/harness'; jest.mock('~/hooks', () => ({ useLocalize: @@ -108,42 +110,41 @@ function renderWithState(args: { const setter = { current: null as null | ((next: SubagentProgress | null) => void) }; let selection: ActiveSubagentPanel | null = null; const SeedHelper = () => { - setter.current = useRecoilCallback( - ({ set }) => - (next: SubagentProgress | null) => - set( - subagentProgressByToolCallId(subagentProgressKey('parent-message', args.toolCallId, 0)), - next, - ), - [], - ); + const store = useStore(); + setter.current = (next: SubagentProgress | null) => + store.set( + subagentProgressByToolCallId(subagentProgressKey('parent-message', args.toolCallId, 0)), + next, + ); return null; }; const SelectionObserver = () => { - selection = useRecoilValue(activeSubagentPanel); + selection = useAtomValue(activeSubagentPanel); return null; }; const rendered = render( - - - - - - - + + + + + + + + + , ); act(() => setter.current?.(args.progress ?? null)); @@ -257,7 +258,7 @@ describe('SubagentCall', () => { it('opens a foreground legacy invocation in the shared panel with persisted activity', () => { let selection: ActiveSubagentPanel | null = null; const Observer = () => { - selection = useRecoilValue(activeSubagentPanel); + selection = useAtomValue(activeSubagentPanel); return null; }; const persistedContent = [ @@ -270,24 +271,26 @@ describe('SubagentCall', () => { ] as Parameters[0]['persistedContent']; render( - - - - - - + + + + + + + + , ); @@ -342,18 +345,20 @@ describe('SubagentCall', () => { render( - - - - - + + + + + + + , ); @@ -361,17 +366,47 @@ describe('SubagentCall', () => { }); it('disables a nested fallback that cannot remain attached to the conversation host', () => { + render( + + + + + + + + + , + ); + + expect(screen.getByRole('button', { name: 'Ran agent' })).toBeDisabled(); + }); + + /** Search results render this same card through `Part`, and that route is not + * a chat surface โ€” there is no panel on it to open. The card still has to + * render; it just cannot be opened from there. */ + it('renders outside a chat surface with its details control disabled', () => { render( diff --git a/client/src/components/Chat/Messages/Content/Wakeup.tsx b/client/src/components/Chat/Messages/Content/Wakeup.tsx index 0a5985f2e2..30cbb68089 100644 --- a/client/src/components/Chat/Messages/Content/Wakeup.tsx +++ b/client/src/components/Chat/Messages/Content/Wakeup.tsx @@ -1,17 +1,17 @@ import { memo, useCallback, useMemo, useState } from 'react'; +import { useRecoilValue } from 'recoil'; import { Button } from '@librechat/client'; import { ChevronDown, Users } from 'lucide-react'; -import { useRecoilValue, useResetRecoilState, useSetRecoilState } from 'recoil'; +import type { ActiveSubagentPanel } from '~/components/Chat/Subagents/state'; import type { WakeupDisplay, WakeupTask } from './Parts/wakeup'; -import type { ActiveSubagentPanel } from '~/store/subagents'; import type { TranslationKeys } from '~/hooks'; import { subagentStatusIcon, subagentStatusLabelKey } from '~/components/Chat/Subagents/status'; import { useParentSubagents } from '~/components/Chat/Subagents/ParentSubagentsProvider'; import { durableSubagentSelection } from '~/components/Chat/Subagents/eventSelection'; import { useLocalize, useExpandCollapse, useLazyCollapseBody } from '~/hooks'; +import { useOpenSubagentPanel } from '~/components/Chat/Subagents/surface'; import { useMCPIconMap, useMCPServerNames } from '~/hooks/MCP'; import { useShareContext } from '~/Providers/ShareContext'; -import { activeSubagentPanel } from '~/store/subagents'; import { cn, getToolDisplayLabel } from '~/utils'; import { useMessageContext } from '~/Providers'; import { StackedToolIcons } from './ToolOutput'; @@ -41,9 +41,7 @@ function WakeupTaskCard({ const { isSharedConvo } = useShareContext(); const { messageId } = useMessageContext(); const { byThreadId } = useParentSubagents(); - const setSelection = useSetRecoilState(activeSubagentPanel); - const setArtifactsVisible = useSetRecoilState(store.artifactsVisibility); - const resetCurrentArtifactId = useResetRecoilState(store.currentArtifactId); + const openPanel = useOpenSubagentPanel(); const child = task.threadId == null ? undefined : byThreadId.get(task.threadId); const selection = useMemo(() => { /** Share pages have no authenticated durable-thread panel; a conversation @@ -83,11 +81,9 @@ function WakeupTaskCard({ const hasResult = task.result.trim() !== ''; const openActivity = useCallback(() => { - if (selection == null) return; - resetCurrentArtifactId(); - setArtifactsVisible(false); - setSelection(selection); - }, [resetCurrentArtifactId, selection, setArtifactsVisible, setSelection]); + if (selection == null || openPanel == null) return; + openPanel(selection); + }, [openPanel, selection]); return (
@@ -99,7 +95,7 @@ function WakeupTaskCard({ /> {title !== '' && {title}} {localize(subagentStatusLabelKey(status))} - {selection != null && ( + {selection != null && openPanel != null && ( /** The trigger identity attributes let the panel's close handler * return keyboard focus to this button. */