From bd24e4548960cbb8dca3fef0c6066c7e851fadeb Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 31 Aug 2026 21:46:58 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=A7=B5=20refactor:=20Give=20the=20Subagen?= =?UTF-8?q?t=20Panel=20One=20Composer=20and=20Main=20Chat's=20Metrics=20(#?= =?UTF-8?q?15436)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ๐Ÿงต refactor: Give the Subagent Panel One Composer and Main Chat's Metrics The panel drifted from main chat in the small measurements a reader notices side by side, and its footer swapped controls under the pointer every time a run settled. - One persistent composer. `@librechat/client` gains a `Composer`: the shared composer surface with a field, a height-holding action row, and the main chat send affordance. The panel renders it for the whole life of a run โ€” Enter steers while the child is live, and continues the thread once it has settled โ€” instead of replacing the input with a "Continue this chat" button at completion. Its placeholder is the main chat one (`Message {agent}`), not bespoke guidance copy. - Continuations carry the draft. Child threads are view-only server-side (`CHILD_THREAD_READ_ONLY_ERROR`), so continuing forks into a real conversation; what the reader typed rides along through the same per-conversation draft store the composer restores from, and comes back in the panel if the fork fails. - Main chat's footer metrics. Turn footers use `messageFooterClasses` and hold their height, so the elapsed timer is sized by the `text-xs` its main chat counterpart inherits rather than the panel's body size, and the reading leaving at completion no longer steps the thread. - The author column matches. Turns render `MessageIcon` unconditionally, and the streaming dot is inset onto the header icon's axis with `underHeaderIcon`, as main chat insets it. - Status indicators pin their color. Label first, dot last, so a longer status grows inboard and the color lands on one column in every row; a live row shimmers its label the way a running tool call does, and the dot no longer pulses against it. - The actor picker is the agent builder's. `ControlCombobox` with agent avatars and search, replacing a bespoke bare `Select`. * ๐Ÿงท fix: Keep Enter Off an Empty Field and Drop Dead Actor Rows - The shared `Composer` never submits an empty field on Enter. Continuing a settled thread needs no text, so its send button stays live โ€” but a stray Enter must not navigate the reader out of the panel. - The actor picker lists only actors with a task to open. Its list has no disabled state, so a child without one was a row that silently did nothing; the selected thread stays listed regardless. - The header glyph loses its plate, like the `MessageRow` author slot, so an agent avatar reads as an avatar rather than an inset badge. The send glyph matches main chat's at 24px. * ๐ŸŽš๏ธ fix: Hand Continuation Text Over in Memory, Not Through Saved Drafts Codex review of the first pass, all three findings. - The continuation's text no longer goes through the composer draft store. Draft restoration is gated on the Save Drafts preference, so with drafts off the forked chat opened empty and the words were left sitting in local storage the reader had asked not to use. A new transient `pendingComposerTextByConvoId` carries them in memory instead, keyed by conversation so the handoff survives a navigation that resolves its record before moving the route; `useTextarea` drains it once, after the autosave restore it must not lose a race with. - The draft is read when the fork lands, not when it was requested, so anything typed while the request was in flight travels too. - Actors whose task id has not arrived are left out of the picker rather than offered as rows that cannot open anything (already in the previous commit; the filter keeps the selected thread listed regardless). * โŒจ๏ธ fix: Give the Panel Composer Main Chat's Keyboard Contract Codex round two. - The composer honors "Press Enter to send". With the preference off a bare Enter is a line break and never reaches the field's action, so reaching for one cannot steer a live run or leave for a continued chat; โŒ˜/Ctrl+Enter submits there instead. - The IME guard covers Safari. `isComposing` is already false on the very Enter that commits a candidate there, so tracked composition state and the legacy `Process`/229 signals stand in for it, as the main chat composer's own guard does. - The actor picker renders in the panel rather than a portal. On mobile the panel is a modal whose focus trap only knows its own descendants, so a portaled search field let Tab escape to the page behind it. * ๐Ÿงญ fix: Bind the Continuation Draft, Contain Escape, Defer to the Key Policy Codex round three. - The continuation draft is bound to the selection that asked for it. The composer is shared across selections, so a fork that lands after the reader moved on was handing over the words now in front of them and clearing the composer they belong to. The live field is still preferred, but only while it is still that continuation's own. - Escape is handled on the panel element instead of through the focus trap's callback. That callback runs on a native listener on the aside, so it fired BEFORE any React handler inside it and closed the whole panel โ€” taking the composer's text โ€” when a nested popover meant to dismiss only itself. As a React handler it bubbles in DOM order, so the actor picker can contain its own Escape; `ControlCombobox` gained an `onOpenChange` so the panel knows when that is. - The composer defers to the host's key policy. `Composer` takes an optional `resolveKeyVerdict`, and the panel routes it through the same `resolveComposerKeyDown`/`useComposerBindings` the main chat form uses, so a rebound or unbound submit shortcut behaves identically here and chords claimed by global shortcuts are left for the window handler. * ๐Ÿ”‘ fix: Stop the Trap Eating Escape, Freeze the Actor Mid-Continuation Codex round four. - `useFocusTrap` only consumes Escape when it actually answers it. Its listener is native and sits on the container, so it runs before every React handler inside โ€” and by calling `preventDefault` with no callback it swallowed the key from the host that had taken Escape back, leaving the panel unclosable. Both other callers pass a handler, so nothing else changes. The panel suite now runs the REAL trap: stubbing it is what let the previous Escape assertions pass in both directions. - Nothing swaps the composer while a continuation is out. The panel holds the draft that travels with the fork, so the actor picker is frozen and the newest-task advance defers until the request settles, rather than leaving another actor's words to be discarded with the panel. - The 4 KiB cap applies only in control mode. It bounds a subagent control command's payload; a continuation is ordinary chat text bound for a composer that caps nothing. * ๐Ÿชข fix: Hold the Selection While a Failed Continuation's Words Survive Codex round five. The newest-task advance deferred while a continuation was in flight, but the error path released that hold at once โ€” so the advance fired, the control identity changed, and the identity effect wiped the composer. The panel stays open after a failed continuation precisely so the reader can retry with those words, and this threw them away. The hold now covers a failed continuation for as long as its draft is still in the box, and releases itself the moment the draft is sent or cleared. The test asserts both halves, and fails if the hold narrows back to the in-flight case. * ๐Ÿงน fix: Hold for Any Unsent Draft, and Let a Cleared One Stay Cleared Codex round six. - Unsent words hold the selection wherever they came from. Protecting the draft only once a request had started or failed left the plainest case open: type a continuation, let a newer task arrive, and the advance changed the control identity and emptied the composer. The hold is now `continuationPending || draft is non-empty`, which subsumes the failed case and lets the extra flag go. - Emptying the field after asking to continue is the reader withdrawing the words. While the composer still belongs to that continuation it IS the draft, verbatim, so the destination no longer reinstates text they deleted; only a composer that has moved to another selection falls back to what was captured at submission. --- .../EventSubagentActivityGroup.test.tsx | 34 + .../Subagents/EventSubagentActivityGroup.tsx | 17 +- .../Chat/Subagents/SubagentActivity.tsx | 18 +- .../Subagents/SubagentConversation.test.tsx | 14 +- .../Chat/Subagents/SubagentConversation.tsx | 35 +- .../Subagents/SubagentThreadPanel.test.tsx | 648 +++++++++++++++--- .../Chat/Subagents/SubagentThreadPanel.tsx | 408 +++++++---- .../src/components/Chat/Subagents/status.ts | 10 +- client/src/hooks/Input/useTextarea.spec.tsx | 38 +- client/src/hooks/Input/useTextarea.ts | 17 +- client/src/hooks/useFocusTrap.ts | 10 +- client/src/locales/en/translation.json | 3 +- client/src/store/families.ts | 20 + .../client/src/components/Composer.spec.tsx | 189 +++++ packages/client/src/components/Composer.tsx | 170 +++++ .../client/src/components/ControlCombobox.tsx | 6 + packages/client/src/components/index.ts | 2 + 17 files changed, 1391 insertions(+), 248 deletions(-) create mode 100644 packages/client/src/components/Composer.spec.tsx create mode 100644 packages/client/src/components/Composer.tsx diff --git a/client/src/components/Chat/Subagents/EventSubagentActivityGroup.test.tsx b/client/src/components/Chat/Subagents/EventSubagentActivityGroup.test.tsx index 3a68f906fc..a1fa1117d0 100644 --- a/client/src/components/Chat/Subagents/EventSubagentActivityGroup.test.tsx +++ b/client/src/components/Chat/Subagents/EventSubagentActivityGroup.test.tsx @@ -220,6 +220,40 @@ describe('EventSubagentActivityGroup', () => { ]); }); + it('pins the status color to one column and shimmers only a live row', () => { + mockChildrenByMessage = new Map([['parent-message', [mockChild, mockCompletedChild]]]); + + render( + + + , + ); + + fireEvent.click(screen.getByRole('button', { name: /com_ui_subagent_activity/ })); + + const rows = [ + screen.getByRole('button', { name: /Visible Agent/ }), + screen.getByRole('button', { name: /Completed Agent/ }), + ]; + const [running, completed] = rows.map((row) => row.lastElementChild as HTMLElement); + + /** Label first, dot last, in every row: a longer status can only grow + * inboard, so the color never moves between rows. */ + expect(running.firstElementChild).toHaveTextContent('com_ui_subagent_thread_status_running'); + expect(running.lastElementChild).toHaveAttribute('aria-hidden', 'true'); + expect(running.lastElementChild).toHaveClass('bg-status-info'); + expect(running.firstElementChild).toHaveClass('shimmer'); + + expect(completed.firstElementChild).toHaveTextContent( + 'com_ui_subagent_thread_status_completed', + ); + expect(completed.lastElementChild).toHaveClass('bg-status-success'); + expect(completed.firstElementChild).not.toHaveClass('shimmer'); + }); + it('does not reopen a child after the user closes it while refresh is pending', async () => { let selection: ActiveSubagentPanel | null = null; let resolveRefresh!: (value: unknown) => void; diff --git a/client/src/components/Chat/Subagents/EventSubagentActivityGroup.tsx b/client/src/components/Chat/Subagents/EventSubagentActivityGroup.tsx index 6d9ff5435d..a31a476248 100644 --- a/client/src/components/Chat/Subagents/EventSubagentActivityGroup.tsx +++ b/client/src/components/Chat/Subagents/EventSubagentActivityGroup.tsx @@ -3,8 +3,8 @@ import { Button, cn } from '@librechat/client'; import { Bot, ChevronDown } from 'lucide-react'; import { useRecoilValue, useResetRecoilState, useSetRecoilState } from 'recoil'; import type { ParentSubagentSummary } from 'librechat-data-provider'; +import { isLiveSubagentStatus, subagentStatusDotClass, subagentStatusLabelKey } from './status'; import { getMessageRowWidthClass } from '~/components/Chat/Messages/ui/MessageRow'; -import { subagentStatusDotClass, subagentStatusLabelKey } from './status'; import { useParentSubagents } from './ParentSubagentsProvider'; import { eventSubagentSelection } from './eventSelection'; import { activeSubagentPanel } from '~/store/subagents'; @@ -217,10 +217,18 @@ function EventSubagentRows({ ) : null} - {/* Fixed-width slot: the dot gives at-a-glance color, the text - keeps a non-color cue, and neither can shift the row as - statuses change length. */} + {/* Fixed-width slot, dot last: the label absorbs every length + change inboard of it, so the color lands on the same pixel + column in every row and cannot drift as statuses change. */} + + {localize(subagentStatusLabelKey(child.status))} + ); diff --git a/client/src/components/Chat/Subagents/SubagentActivity.tsx b/client/src/components/Chat/Subagents/SubagentActivity.tsx index da435f03fa..262531949b 100644 --- a/client/src/components/Chat/Subagents/SubagentActivity.tsx +++ b/client/src/components/Chat/Subagents/SubagentActivity.tsx @@ -7,7 +7,12 @@ import { CheckCircle2, Clock3, Maximize2, Minimize2, XCircle } from 'lucide-reac import type { TMessageContentParts } from 'librechat-data-provider'; import type { ChildActivity, ChildActivityItem } from './adapters'; import type { TranslationKeys } from '~/hooks'; -import { isAbnormalTerminalStatus, subagentStatusIcon, subagentStatusLabelKey } from './status'; +import { + isAbnormalTerminalStatus, + isLiveSubagentStatus, + subagentStatusIcon, + subagentStatusLabelKey, +} from './status'; import MarkdownLite from '~/components/Chat/Messages/Content/MarkdownLite'; import ContentParts from '~/components/Chat/Messages/Content/ContentParts'; import Container from '~/components/Chat/Messages/Content/Container'; @@ -297,6 +302,7 @@ export function SubagentActivityContent({ state = 'ready', showPrompt = true, conversationId = null, + underHeaderIcon = false, onCancelControl, }: { activity: ChildActivity; @@ -304,17 +310,21 @@ export function SubagentActivityContent({ state?: 'ready' | 'loading' | 'error'; showPrompt?: boolean; conversationId?: string | null; + /** Set when this body sits directly beneath a `MessageRow` author header, so + * the streaming dot centers on the header icon's axis as it does in main + * chat (see `EmptyTextPart`). */ + underHeaderIcon?: boolean; onCancelControl?: (controlId: string) => void; }) { const localize = useLocalize(); - const isSubmitting = activity.status === 'running' || activity.status === 'dispatched'; + const isSubmitting = isLiveSubagentStatus(activity.status); const parts = useMemo(() => activity.items.map(toContentPart), [activity.items]); let body: React.ReactNode; if (state === 'loading') { body = ( - + ); } else if (state === 'error') { @@ -326,7 +336,7 @@ export function SubagentActivityContent({ } else if (activity.items.length === 0) { body = isSubmitting ? ( - + ) : null; } else { diff --git a/client/src/components/Chat/Subagents/SubagentConversation.test.tsx b/client/src/components/Chat/Subagents/SubagentConversation.test.tsx index 3ff3a6990c..0c6278c352 100644 --- a/client/src/components/Chat/Subagents/SubagentConversation.test.tsx +++ b/client/src/components/Chat/Subagents/SubagentConversation.test.tsx @@ -40,7 +40,14 @@ jest.mock('~/components/Chat/Messages/Content/Container', () => ({ })); jest.mock('~/components/Chat/Messages/Content/Parts', () => ({ - EmptyText: () =>
, + EmptyText: ({ underHeaderIcon }: { underHeaderIcon?: boolean }) => ( +
+ ), +})); + +jest.mock('~/components/Chat/Messages/MessageIcon', () => ({ + __esModule: true, + default: () => , })); jest.mock('lucide-react', () => ({ @@ -114,7 +121,10 @@ describe('SubagentConversation', () => { expect(screen.getByText('The release is ready.')).toBeInTheDocument(); expect(screen.queryByText('com_ui_subagent_thread_status_completed')).not.toBeInTheDocument(); expect(screen.queryByText('com_ui_subagent_thread_status_running')).not.toBeInTheDocument(); - expect(screen.getByTestId('thinking-cursor')).toBeInTheDocument(); + /** Both halves of the main chat author column: the shared glyph, and the + * streaming dot inset onto that glyph's axis the way main chat insets it. */ + expect(screen.getAllByTestId('message-icon')).toHaveLength(2); + expect(screen.getByTestId('thinking-cursor')).toHaveAttribute('data-under-header-icon', 'true'); expect(container.querySelectorAll('.message-render')).toHaveLength(4); expect(container.querySelectorAll('.user-turn')).toHaveLength(2); expect(container.querySelectorAll('.agent-turn')).toHaveLength(2); diff --git a/client/src/components/Chat/Subagents/SubagentConversation.tsx b/client/src/components/Chat/Subagents/SubagentConversation.tsx index 29982d619e..e9da61eeda 100644 --- a/client/src/components/Chat/Subagents/SubagentConversation.tsx +++ b/client/src/components/Chat/Subagents/SubagentConversation.tsx @@ -1,7 +1,7 @@ import { useMemo, useState } from 'react'; import { useRecoilValue } from 'recoil'; +import { ChevronDown, CornerDownRight, Radio } from 'lucide-react'; import { ContentTypes, EModelEndpoint } from 'librechat-data-provider'; -import { Bot, ChevronDown, CornerDownRight, Radio } from 'lucide-react'; import { Button, Collapsible, CollapsibleContent, CollapsibleTrigger } from '@librechat/client'; import type { TMessageContentParts } from 'librechat-data-provider'; import type { ReactNode } from 'react'; @@ -9,12 +9,14 @@ import type { ChildConversationTurn } from './adapters'; import type { TranslationKeys } from '~/hooks'; import { SubagentActivityContent, SubagentStatus } from './SubagentActivity'; import ContentParts from '~/components/Chat/Messages/Content/ContentParts'; +import { isAbnormalTerminalStatus, isLiveSubagentStatus } from './status'; +import { messageFooterClasses } from '~/components/Chat/Messages/styles'; import MessageRow from '~/components/Chat/Messages/ui/MessageRow'; import { ElapsedTimer } from '~/components/Chat/Messages/Elapsed'; import MessageIcon from '~/components/Chat/Messages/MessageIcon'; -import { isAbnormalTerminalStatus } from './status'; import { useAgentsMapContext } from '~/Providers'; import { useLocalize } from '~/hooks'; +import { cn } from '~/utils'; import store from '~/store'; const TRIGGER_LABELS = { @@ -201,14 +203,21 @@ function ChildMessage({ } else { limitedNotice = localize('com_ui_subagent_activity_details_unavailable'); } - let footer: ReactNode = null; + let footerContent: ReactNode = null; if (isAbnormalTerminalStatus(turn.activity.status)) { - footer = ; - } else if (turn.activity.status === 'running' || turn.activity.status === 'dispatched') { + footerContent = ; + } else if (isLiveSubagentStatus(turn.activity.status)) { const triggeredAt = turn.trigger.createdAt ?? turn.trigger.externalEvent?.occurredAt; const startedAt = triggeredAt == null ? NaN : Date.parse(triggeredAt); - footer = ; + footerContent = ; } + /** The main chat footer's own metrics, held whether or not anything occupies + * the slot: the timer leaving at completion must not step the turns below it + * upward, and the reading has to be sized by the same `text-xs` its main + * chat counterpart inherits rather than by the panel's body size. */ + const footer = ( +
{footerContent}
+ ); const iconData = { endpoint: EModelEndpoint.agents, modelLabel: label, @@ -217,15 +226,10 @@ function ChildMessage({ return ( - - - ) : ( - - ) - } + /** The main chat author glyph, unconditionally: with no resolved agent it + * falls back to the endpoint icon there too, so an unresolved child does + * not get a differently-inset placeholder of its own. */ + icon={} label={label} footer={footer} ariaLabel={label} @@ -239,6 +243,7 @@ function ChildMessage({ state={state} showPrompt={false} conversationId={conversationId} + underHeaderIcon onCancelControl={onCancelControl} /> {detailsLimited && detailState !== 'loading' && ( diff --git a/client/src/components/Chat/Subagents/SubagentThreadPanel.test.tsx b/client/src/components/Chat/Subagents/SubagentThreadPanel.test.tsx index 51e1dfbf65..244a3fd4a1 100644 --- a/client/src/components/Chat/Subagents/SubagentThreadPanel.test.tsx +++ b/client/src/components/Chat/Subagents/SubagentThreadPanel.test.tsx @@ -15,6 +15,8 @@ import { } from '~/store/subagents'; import { initSubagentAggregatorState, initSubagentTickerState } from '~/utils/subagentContent'; import SubagentThreadPanel from './SubagentThreadPanel'; +import { getDraft } from '~/utils'; +import store from '~/store'; const mockUseSubagentThreadQuery = jest.fn(); const mockUseSubagentActivityStream = jest.fn(); @@ -75,7 +77,10 @@ jest.mock('~/data-provider/Subagents/useSubagentActivityStream', () => ({ })); jest.mock('~/hooks', () => ({ - useFocusTrap: jest.fn(), + /** The real trap: its Escape listener is native and sits on the panel, so it + * runs before the panel's own React handler. Stubbing it made the panel's + * Escape assertions pass in both directions. */ + useFocusTrap: jest.requireActual('~/hooks/useFocusTrap').default, useLocalize: () => (key: string) => key, useNavigateToConvo: () => ({ navigateToConvo: mockNavigateToConvo }), })); @@ -197,69 +202,140 @@ jest.mock('./SubagentConversation', () => ({ ), })); -jest.mock('@librechat/client', () => { - const mockReact = jest.requireActual('react'); - const MockSelectContext = mockReact.createContext((_value: string): void => {}); - return { - Alert: ({ children, ...props }: React.ComponentProps<'div'>) => ( -
- {children} -
- ), - Button: ({ children, ...props }: React.ComponentProps<'button'>) => ( - - ), - Select: ({ - children, - onValueChange, - }: { - children: React.ReactNode; - onValueChange: (value: string) => void; - }) => {children}, - SelectTrigger: ({ children, ...props }: React.ComponentProps<'button'>) => ( - + ), + Skeleton: () => null, + ControlCombobox: ({ + items, + setValue, + ariaLabel, + displayValue, + onOpenChange, + disabled, + }: { + items: { value: string; label: string }[]; + setValue: (value: string) => void; + ariaLabel: string; + displayValue?: string; + onOpenChange?: (open: boolean) => void; + disabled?: boolean; + }) => ( +
+ - ), - SelectValue: () => null, - SelectContent: ({ children }: { children: React.ReactNode }) => ( -
{children}
- ), - SelectItem: ({ - value, - children, - ...props - }: React.ComponentProps<'button'> & { value: string }) => { - const onValueChange = mockReact.useContext(MockSelectContext); - return ( - - ); - }, - composerSurfaceClasses: () => '', - composerSurfaceShadow: { focused: '', blurred: '', within: '' }, - TextareaAutosize: ({ - minRows: _minRows, - maxRows: _maxRows, - ...props - }: React.ComponentProps<'textarea'> & { minRows?: number; maxRows?: number }) => ( -