From 9b4ae068b2c610cc77653f60676cee746807387a Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Thu, 16 Apr 2026 17:27:32 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=92=B2=20feat:=20Manual=20Skill=20Invocat?= =?UTF-8?q?ion=20via=20$=20Command=20Popover=20(UI=20only)=20(#12690)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add $ command popover for manual skill invocation Adds a new `$` command trigger in the chat textarea that opens a searchable popover listing user-invocable skills. Selecting a skill inserts `$skill-name` into the message and enables the skills badge on the ephemeral agent. Follows the same patterns as `@` mentions and `/` prompts. * test: update useHandleKeyUp tests for $ skill command Add showSkillsPopoverFamily, dollarCommand, and SKILLS permission to the store/recoil/access mocks. Include test cases for the $ trigger, toggle gating, and permission gating. * fix: address review findings in SkillsCommand - Exclude auto-mode skills from popover (isUserInvocable now returns false for InvocationMode.auto) - Rewrite JSDoc to match the corrected filter logic - Guard ArrowUp/ArrowDown against NaN when matches is empty - Replace useRecoilState with useSetRecoilState for ephemeralAgent to avoid subscribing to unrelated agent state changes - Move skills guard into setter callback and drop ephemeralAgent from handleSelect dependency array - Add e.preventDefault() for Tab key alongside Enter - Render error state (com_ui_skills_load_error) on query failure - Render empty state (com_ui_skills_empty) when no matches - Hoist ScrollText icon to module-level constant - Export isUserInvocable for testability * fix: address follow-up review findings - Differentiate empty-catalog ("No skills yet") from no-match search ("No skills found") using existing com_ui_no_skills_found - Clamp activeIndex when matches shrink from search filtering to prevent silent Enter/Tab failures - Add early return after Escape handler to skip redundant checks - Reorder package imports shortest-to-longest after react - Add fast-typing test case for $ command ("$sk" at position 3) * fix: gate $ command on assistants endpoint, fix Tab on empty matches - Block $ popover on assistants/azureAssistants endpoints where ephemeralAgent is not sent in the submission payload - Allow Tab to close the popover when matches is empty instead of trapping keyboard focus - Add endpoint gating tests for $ on both assistants endpoints * fix: reset skills popover on assistants switch + paginate skills query - Close $ skills popover when endpoint switches to assistants or azureAssistants, mirroring the existing + command reset - Switch SkillsCommand from a single-page list query to the cursor-paginated useSkillsInfiniteQuery and auto-fetch all pages so client-side search covers the full catalog instead of only the first 100 entries - Show the spinner during background page fetches and suppress the empty-state copy until paging completes - Add test for popover reset on endpoint switch * fix: prevent currency hijack and form submit on $ command - Reject the $ trigger when fast-typed text after $ does not start with a lowercase letter, so currency input like $100 or $5.99 no longer opens the skills popover and clears the textarea - preventDefault() on Enter when the popover has no matches so the surrounding chat form does not submit when the user dismisses the popover via Enter - Add tests for $100 and $5.99 currency inputs * fix: defer $ popover to second keystroke and stop pagination on errors - Defer opening the skills popover until a letter follows the $ character. Bare $ no longer triggers the popover, so starting a message with $100, $5.99, or $EUR is fully preserved (the textarea is not cleared by useInitPopoverInput on the first keystroke). $a, $skill, $my-skill still open as expected. - Add a sticky paginationBlockedRef circuit breaker on the auto fetchNextPage effect so a transient page request error cannot spin into an unbounded retry loop when isError flips back to false on the next attempt. - Update tests: bare $ no longer triggers, $a does, currency cases remain blocked. * feat: scaffold structured manual-skill channel for follow-up PR Add a per-conversation pendingManualSkillsByConvoId atom family that SkillsCommand appends to on selection. This is the writer half of the structured channel that will let a follow-up PR deterministically prime SKILL.md as a meta user message before the LLM turn (mirroring Claude Code's `/skill` invocation), so the backend never has to regex-parse `$name` out of user text. The submit pipeline does not yet read this atom; the textual `$skill-name ` insertion remains the authoritative signal until the follow-up wires the read + reset on submit. Reset is already wired into useClearStates so the atom does not leak across conversation switches. * test: cover SkillsCommand selection-flow contract Add a component test that locks in the contract the follow-up manualSkills PR has to honor when a user picks a skill in the $ popover: - Pushes the skill name onto pendingManualSkillsByConvoId (the per-conversation structured channel), with dedup - Flips ephemeralAgent.skills to true via the callback-form setter - Inserts $skill-name into the textarea as cosmetic confirmation - Closes the popover via setShowSkillsPopover(false) - Renders nothing when the popover atom is false Mocks recoil setters and the skills query so the test exercises the real component logic without spinning up the full provider stack. * revert: drop $ defer-until-letter guard for sibling consistency Bring $ in line with @, /, and +: open the popover on the bare trigger character. The earlier defer guard kept currency input like \$100 from clobbering the textarea, but it broke the natural "type \$ to browse skills" UX and was inconsistent with the other trigger chars, none of which gate on the follow-up character (/path/to/file, @username, +1 all clear the textarea the same way). The currency hijack remains recoverable via Escape. Drop the corresponding paste-protection tests for \$100 and \$5.99, restore the bare-\$ trigger test. --- client/src/components/Chat/Input/ChatForm.tsx | 2 + .../src/components/Chat/Input/MentionItem.tsx | 2 +- .../components/Chat/Input/SkillsCommand.tsx | 331 ++++++++++++++++++ .../Input/__tests__/SkillsCommand.spec.tsx | 187 ++++++++++ client/src/hooks/Config/useClearStates.ts | 2 + client/src/hooks/Input/useHandleKeyUp.spec.ts | 84 ++++- client/src/hooks/Input/useHandleKeyUp.ts | 21 +- client/src/locales/en/translation.json | 2 + client/src/store/families.ts | 22 ++ client/src/store/settings.ts | 1 + 10 files changed, 650 insertions(+), 4 deletions(-) create mode 100644 client/src/components/Chat/Input/SkillsCommand.tsx create mode 100644 client/src/components/Chat/Input/__tests__/SkillsCommand.spec.tsx diff --git a/client/src/components/Chat/Input/ChatForm.tsx b/client/src/components/Chat/Input/ChatForm.tsx index 87ceccdc71..50f2076dd0 100644 --- a/client/src/components/Chat/Input/ChatForm.tsx +++ b/client/src/components/Chat/Input/ChatForm.tsx @@ -26,6 +26,7 @@ import AttachFileChat from './Files/AttachFileChat'; import FileFormChat from './Files/FileFormChat'; import { cn, removeFocusRings } from '~/utils'; import TextareaHeader from './TextareaHeader'; +import SkillsCommand from './SkillsCommand'; import PromptsCommand from './PromptsCommand'; import AudioRecorder from './AudioRecorder'; import CollapseChat from './CollapseChat'; @@ -254,6 +255,7 @@ const ChatForm = memo(function ChatForm({ textAreaRef={textAreaRef} /> +
) => void; index: number; - type?: 'prompt' | 'mention' | 'add-convo'; + type?: 'prompt' | 'mention' | 'add-convo' | 'skill'; icon?: React.ReactNode; isActive?: boolean; description?: string; diff --git a/client/src/components/Chat/Input/SkillsCommand.tsx b/client/src/components/Chat/Input/SkillsCommand.tsx new file mode 100644 index 0000000000..1ed63fafaf --- /dev/null +++ b/client/src/components/Chat/Input/SkillsCommand.tsx @@ -0,0 +1,331 @@ +import { memo, useState, useRef, useEffect, useMemo, useCallback } from 'react'; +import { ScrollText } from 'lucide-react'; +import { AutoSizer, List } from 'react-virtualized'; +import { Spinner, useCombobox } from '@librechat/client'; +import { InvocationMode } from 'librechat-data-provider'; +import { useRecoilValue, useSetRecoilState } from 'recoil'; +import type { TSkillSummary } from 'librechat-data-provider'; +import type { MentionOption } from '~/common'; +import useInitPopoverInput from '~/hooks/Input/useInitPopoverInput'; +import { useSkillsInfiniteQuery } from '~/data-provider'; +import { ephemeralAgentByConvoId } from '~/store'; +import { removeCharIfLast } from '~/utils'; +import MentionItem from './MentionItem'; +import { useLocalize } from '~/hooks'; +import store from '~/store'; + +const commandChar = '$'; +const ROW_HEIGHT = 44; +const skillIcon = ; + +/** + * Determines whether a skill should appear in the `$` command popover. + * `manual` and `both` are user-invocable. `auto` is model-only and hidden. + * Skills without an explicit mode (undefined) default to visible for + * backward compatibility until the backend persists `invocationMode`. + */ +export function isUserInvocable(skill: TSkillSummary): boolean { + const mode = skill.invocationMode; + if (mode == null || mode === InvocationMode.both) { + return true; + } + return mode === InvocationMode.manual; +} + +function SkillsCommandContent({ + index, + textAreaRef, + conversationId, +}: { + index: number; + textAreaRef: React.MutableRefObject; + conversationId: string; +}) { + const localize = useLocalize(); + const setShowSkillsPopover = useSetRecoilState(store.showSkillsPopoverFamily(index)); + const setEphemeralAgent = useSetRecoilState(ephemeralAgentByConvoId(conversationId)); + const setPendingManualSkills = useSetRecoilState( + store.pendingManualSkillsByConvoId(conversationId), + ); + + const { data, isLoading, isError, fetchNextPage, hasNextPage, isFetchingNextPage } = + useSkillsInfiniteQuery({ limit: 50 }); + + /* Sticky circuit breaker: once any page request fails, stop auto-fetching + for the lifetime of the popover so a transient API error does not turn + into an unbounded retry loop (isError can flip back to false on the + next attempt, which would otherwise re-arm the auto-fetch effect). */ + const paginationBlockedRef = useRef(false); + useEffect(() => { + if (isError) { + paginationBlockedRef.current = true; + } + }, [isError]); + + /* Auto-fetch all pages so client-side search covers the full catalog, + not just the first page. The skills API is server-side capped. */ + useEffect(() => { + if (paginationBlockedRef.current || isError) { + return; + } + if (hasNextPage && !isFetchingNextPage) { + fetchNextPage(); + } + }, [hasNextPage, isFetchingNextPage, isError, fetchNextPage]); + + const skillOptions: MentionOption[] = useMemo(() => { + if (!data?.pages) { + return []; + } + return data.pages.reduce((acc, page) => { + for (const skill of page.skills) { + if (isUserInvocable(skill)) { + acc.push({ + label: skill.displayTitle ?? skill.name, + value: skill.name, + description: skill.description, + type: 'skill', + icon: skillIcon, + }); + } + } + return acc; + }, []); + }, [data?.pages]); + + const [activeIndex, setActiveIndex] = useState(0); + const timeoutRef = useRef(null); + const inputRef = useRef(null); + + const { open, setOpen, searchValue, setSearchValue, matches } = useCombobox({ + value: '', + options: skillOptions, + }); + + const initInputRef = useInitPopoverInput({ + inputRef, + textAreaRef, + commandChar, + setSearchValue, + setOpen, + }); + + const handleSelect = useCallback( + (mention?: MentionOption) => { + if (!mention) { + return; + } + + setSearchValue(''); + setOpen(false); + setShowSkillsPopover(false); + + if (textAreaRef.current) { + removeCharIfLast(textAreaRef.current, commandChar); + } + + setEphemeralAgent((prev) => { + if (prev?.skills) { + return prev; + } + return { ...(prev || {}), skills: true }; + }); + + /* Structured channel for manual skill invocations. The follow-up PR + will read this in the submit pipeline and prime the corresponding + SKILL.md as a meta user message before the LLM turn, mirroring + Claude Code's `/skill` deterministic injection. The textual + `$skill-name ` insertion below remains as user-visible confirmation + and is treated as cosmetic by that future pipeline. */ + setPendingManualSkills((prev) => + prev.includes(mention.value) ? prev : [...prev, mention.value], + ); + + const textarea = textAreaRef.current; + if (textarea) { + const insertion = `$${mention.value} `; + const nativeInputValueSetter = Object.getOwnPropertyDescriptor( + HTMLTextAreaElement.prototype, + 'value', + )?.set; + if (nativeInputValueSetter) { + nativeInputValueSetter.call(textarea, insertion); + textarea.dispatchEvent(new Event('input', { bubbles: true })); + } + textarea.focus(); + textarea.setSelectionRange(insertion.length, insertion.length); + } + }, + [ + setSearchValue, + setOpen, + setShowSkillsPopover, + textAreaRef, + setEphemeralAgent, + setPendingManualSkills, + ], + ); + + useEffect(() => { + if (!open) { + setActiveIndex(0); + } + }, [open]); + + useEffect(() => { + setActiveIndex((prev) => Math.min(prev, Math.max(matches.length - 1, 0))); + }, [matches.length]); + + useEffect(() => { + return () => { + if (timeoutRef.current) { + clearTimeout(timeoutRef.current); + } + }; + }, []); + + useEffect(() => { + const el = document.getElementById(`skill-item-${activeIndex}`); + el?.scrollIntoView({ behavior: 'instant', block: 'nearest' }); + }, [activeIndex]); + + const rowRenderer = ({ + index, + key, + style, + }: { + index: number; + key: string; + style: React.CSSProperties; + }) => { + const mention = matches[index] as MentionOption; + return ( + { + if (timeoutRef.current) { + clearTimeout(timeoutRef.current); + } + timeoutRef.current = null; + handleSelect(mention); + }} + name={mention.label ?? ''} + icon={mention.icon} + description={mention.description} + isActive={index === activeIndex} + /> + ); + }; + + return ( +
+
+ { + if (e.key === 'Escape') { + setOpen(false); + setShowSkillsPopover(false); + textAreaRef.current?.focus(); + return; + } + if (e.key === 'ArrowDown') { + if (matches.length === 0) { + return; + } + setActiveIndex((prevIndex) => (prevIndex + 1) % matches.length); + } else if (e.key === 'ArrowUp') { + if (matches.length === 0) { + return; + } + setActiveIndex((prevIndex) => (prevIndex - 1 + matches.length) % matches.length); + } else if (e.key === 'Enter' || e.key === 'Tab') { + if (matches.length === 0) { + if (e.key === 'Enter') { + e.preventDefault(); + } + setOpen(false); + setShowSkillsPopover(false); + textAreaRef.current?.focus(); + return; + } + e.preventDefault(); + handleSelect(matches[activeIndex] as MentionOption | undefined); + } else if (e.key === 'Backspace' && searchValue === '') { + setOpen(false); + setShowSkillsPopover(false); + textAreaRef.current?.focus(); + } + }} + onChange={(e) => setSearchValue(e.target.value)} + onFocus={() => setOpen(true)} + onBlur={() => { + timeoutRef.current = setTimeout(() => { + setOpen(false); + setShowSkillsPopover(false); + }, 150); + }} + /> + {open && (isLoading || isFetchingNextPage) && matches.length === 0 && ( +
+ +
+ )} + {open && isError && ( +
+ {localize('com_ui_skills_load_error')} +
+ )} + {open && !isLoading && !isFetchingNextPage && !isError && matches.length === 0 && ( +
+ {localize(searchValue ? 'com_ui_no_skills_found' : 'com_ui_skills_empty')} +
+ )} + {open && matches.length > 0 && ( +
+ + {({ width }) => ( + + )} + +
+ )} +
+
+ ); +} + +const SkillsCommand = memo(function SkillsCommand({ + index, + textAreaRef, + conversationId, +}: { + index: number; + textAreaRef: React.MutableRefObject; + conversationId: string; +}) { + const show = useRecoilValue(store.showSkillsPopoverFamily(index)); + if (!show) { + return null; + } + return ( + + ); +}); + +export default SkillsCommand; diff --git a/client/src/components/Chat/Input/__tests__/SkillsCommand.spec.tsx b/client/src/components/Chat/Input/__tests__/SkillsCommand.spec.tsx new file mode 100644 index 0000000000..27d665a674 --- /dev/null +++ b/client/src/components/Chat/Input/__tests__/SkillsCommand.spec.tsx @@ -0,0 +1,187 @@ +/** + * Locks in the selection-flow contract that the follow-up `manualSkills` + * PR has to honor: when a user picks a skill in the `$` popover the + * component must (a) push the skill name onto the per-conversation + * `pendingManualSkillsByConvoId` atom, (b) flip `ephemeralAgent.skills` + * to true, and (c) insert `$skill-name ` into the textarea. + */ +import React from 'react'; +import { act } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { render, screen } from '@testing-library/react'; + +const CONVO_ID = 'convo-1'; + +const mockSetShowSkillsPopover = jest.fn(); +const mockSetEphemeralAgent = jest.fn(); +const mockSetPendingManualSkills = jest.fn(); +const mockShowSkillsPopover = { current: true }; + +jest.mock('recoil', () => { + const actual = jest.requireActual('recoil'); + return { + ...actual, + useRecoilValue: jest.fn((atom: unknown) => { + if (atom === 'show-skills-popover') { + return mockShowSkillsPopover.current; + } + return undefined; + }), + useRecoilState: jest.fn(() => [null, jest.fn()]), + useSetRecoilState: jest.fn((atom: unknown) => { + if (atom === 'show-skills-popover') { + return mockSetShowSkillsPopover; + } + if (atom === 'ephemeral-agent') { + return mockSetEphemeralAgent; + } + if (atom === 'pending-manual-skills') { + return mockSetPendingManualSkills; + } + return jest.fn(); + }), + }; +}); + +jest.mock('~/store', () => ({ + __esModule: true, + default: { + showSkillsPopoverFamily: () => 'show-skills-popover', + pendingManualSkillsByConvoId: () => 'pending-manual-skills', + }, + ephemeralAgentByConvoId: () => 'ephemeral-agent', + pendingManualSkillsByConvoId: () => 'pending-manual-skills', +})); + +const mockUseSkillsInfiniteQuery = jest.fn(); +jest.mock('~/data-provider', () => ({ + useSkillsInfiniteQuery: () => mockUseSkillsInfiniteQuery(), +})); + +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string) => key, +})); + +jest.mock('@librechat/client', () => { + const actual = jest.requireActual('@librechat/client'); + return { + ...actual, + Spinner: () => null, + }; +}); + +/* react-virtualized renders nothing in jsdom without measured size; replace + AutoSizer + List with a flat ul so row clicks are exercised normally. */ +jest.mock('react-virtualized', () => ({ + ...jest.requireActual('react-virtualized'), + AutoSizer: ({ children }: { children: (size: { width: number }) => React.ReactNode }) => + children({ width: 320 }), + List: ({ + rowCount, + rowRenderer, + }: { + rowCount: number; + rowRenderer: (args: { + index: number; + key: string; + style: React.CSSProperties; + }) => React.ReactNode; + }) => { + const rows: React.ReactNode[] = []; + for (let i = 0; i < rowCount; i++) { + rows.push(rowRenderer({ index: i, key: `row-${i}`, style: {} })); + } + return
    {rows}
; + }, +})); + +import SkillsCommand from '../SkillsCommand'; + +const makeTextarea = (initial = '$') => { + const textarea = document.createElement('textarea'); + textarea.value = initial; + document.body.appendChild(textarea); + return { current: textarea } as React.MutableRefObject; +}; + +const skillsResponse = { + pages: [ + { + skills: [ + { + _id: '1', + name: 'brand-guidelines', + displayTitle: 'Brand Guidelines', + description: 'Apply brand styling', + author: 'u', + authorName: 'U', + version: 1, + source: 'inline', + fileCount: 0, + createdAt: '', + updatedAt: '', + }, + ], + has_more: false, + after: null, + }, + ], +}; + +beforeEach(() => { + jest.clearAllMocks(); + document.body.innerHTML = ''; + mockShowSkillsPopover.current = true; + mockUseSkillsInfiniteQuery.mockReturnValue({ + data: skillsResponse, + isLoading: false, + isError: false, + fetchNextPage: jest.fn(), + hasNextPage: false, + isFetchingNextPage: false, + }); +}); + +describe('SkillsCommand', () => { + it('renders nothing when the popover atom is false', () => { + mockShowSkillsPopover.current = false; + const textAreaRef = makeTextarea(); + const { container } = render( + , + ); + expect(container).toBeEmptyDOMElement(); + }); + + it('selecting a skill pushes to pendingManualSkillsByConvoId, flips ephemeralAgent.skills, inserts $name into textarea, and closes the popover', async () => { + const user = userEvent.setup(); + const textAreaRef = makeTextarea('$'); + render(); + + const skillButton = await screen.findByRole('button', { name: /Brand Guidelines/i }); + await act(async () => { + await user.click(skillButton); + }); + + /* Structured channel: the skill name is pushed into the per-convo atom, + which is the contract the follow-up PR depends on. */ + expect(mockSetPendingManualSkills).toHaveBeenCalledTimes(1); + const updater = mockSetPendingManualSkills.mock.calls[0][0] as (prev: string[]) => string[]; + expect(updater([])).toEqual(['brand-guidelines']); + expect(updater(['brand-guidelines'])).toEqual(['brand-guidelines']); + + /* Ephemeral agent gets skills enabled so the badge lights up and the + backend includes the skill catalog. */ + expect(mockSetEphemeralAgent).toHaveBeenCalledTimes(1); + const agentUpdater = mockSetEphemeralAgent.mock.calls[0][0] as ( + prev: { skills?: boolean } | null, + ) => { skills?: boolean }; + expect(agentUpdater(null)).toEqual({ skills: true }); + expect(agentUpdater({ skills: true })).toEqual({ skills: true }); + + /* Cosmetic textarea insertion remains in place for user feedback. */ + expect(textAreaRef.current?.value).toBe('$brand-guidelines '); + + /* Popover dismisses on selection. */ + expect(mockSetShowSkillsPopover).toHaveBeenCalledWith(false); + }); +}); diff --git a/client/src/hooks/Config/useClearStates.ts b/client/src/hooks/Config/useClearStates.ts index 140f83e924..46ec9476c7 100644 --- a/client/src/hooks/Config/useClearStates.ts +++ b/client/src/hooks/Config/useClearStates.ts @@ -32,6 +32,8 @@ export default function useClearStates() { reset(store.showMentionPopoverFamily(key)); reset(store.showPlusPopoverFamily(key)); reset(store.showPromptsPopoverFamily(key)); + reset(store.showSkillsPopoverFamily(key)); + reset(store.pendingManualSkillsByConvoId(key.toString())); reset(store.activePromptByIndex(key)); reset(store.globalAudioURLFamily(key)); reset(store.globalAudioFetchingFamily(key)); diff --git a/client/src/hooks/Input/useHandleKeyUp.spec.ts b/client/src/hooks/Input/useHandleKeyUp.spec.ts index 79bbae5994..c9ea83ea90 100644 --- a/client/src/hooks/Input/useHandleKeyUp.spec.ts +++ b/client/src/hooks/Input/useHandleKeyUp.spec.ts @@ -1,10 +1,12 @@ const mockSetShowMentionPopover = jest.fn(); const mockSetShowPlusPopover = jest.fn(); const mockSetShowPromptsPopover = jest.fn(); +const mockSetShowSkillsPopover = jest.fn(); const mockHasPromptsAccess = { current: true }; const mockHasMultiConvoAccess = { current: true }; +const mockHasSkillsAccess = { current: true }; const mockEndpoint = { current: 'openAI' as string | null }; -const mockCommandToggles = { at: true, plus: true, slash: true }; +const mockCommandToggles = { at: true, plus: true, slash: true, dollar: true }; jest.mock('recoil', () => ({ ...jest.requireActual('recoil'), @@ -24,6 +26,9 @@ jest.mock('recoil', () => ({ if (atom === 'slashCommand') { return mockCommandToggles.slash; } + if (atom === 'dollarCommand') { + return mockCommandToggles.dollar; + } return undefined; }), useSetRecoilState: jest.fn((atom: string) => { @@ -36,6 +41,9 @@ jest.mock('recoil', () => ({ if (atom === 'showPromptsPopoverFamily-0') { return mockSetShowPromptsPopover; } + if (atom === 'showSkillsPopoverFamily-0') { + return mockSetShowSkillsPopover; + } return jest.fn(); }), })); @@ -44,11 +52,13 @@ jest.mock('~/store', () => ({ showPromptsPopoverFamily: (idx: number) => `showPromptsPopoverFamily-${idx}`, showMentionPopoverFamily: (idx: number) => `showMentionPopoverFamily-${idx}`, showPlusPopoverFamily: (idx: number) => `showPlusPopoverFamily-${idx}`, + showSkillsPopoverFamily: (idx: number) => `showSkillsPopoverFamily-${idx}`, effectiveEndpointByIndex: (idx: number) => `effectiveEndpointByIndex-${idx}`, latestMessageFamily: (idx: number) => `latestMessageFamily-${idx}`, atCommand: 'atCommand', plusCommand: 'plusCommand', slashCommand: 'slashCommand', + dollarCommand: 'dollarCommand', })); jest.mock('~/hooks/Roles/useHasAccess', () => @@ -59,6 +69,9 @@ jest.mock('~/hooks/Roles/useHasAccess', () => if (permissionType === 'MULTI_CONVO') { return mockHasMultiConvoAccess.current; } + if (permissionType === 'SKILLS') { + return mockHasSkillsAccess.current; + } return false; }), ); @@ -96,6 +109,7 @@ const renderUseHandleKeyUp = ( setShowMentionPopover: mockSetShowMentionPopover, setShowPlusPopover: mockSetShowPlusPopover, setShowPromptsPopover: mockSetShowPromptsPopover, + setShowSkillsPopover: mockSetShowSkillsPopover, }; }; @@ -103,10 +117,12 @@ beforeEach(() => { jest.clearAllMocks(); mockHasPromptsAccess.current = true; mockHasMultiConvoAccess.current = true; + mockHasSkillsAccess.current = true; mockEndpoint.current = 'openAI'; mockCommandToggles.at = true; mockCommandToggles.plus = true; mockCommandToggles.slash = true; + mockCommandToggles.dollar = true; }); describe('useHandleKeyUp', () => { @@ -137,6 +153,15 @@ describe('useHandleKeyUp', () => { expect(setShowPlusPopover).toHaveBeenCalledWith(true); }); + + it('triggers $ skill command for "$" at position 1', () => { + const ref = makeTextAreaRef('$', 1); + const { handleKeyUp, setShowSkillsPopover } = renderUseHandleKeyUp(ref); + + act(() => handleKeyUp(makeKeyEvent('$'))); + + expect(setShowSkillsPopover).toHaveBeenCalledWith(true); + }); }); describe('fast typing — cursor past position 1 but text is short', () => { @@ -167,6 +192,15 @@ describe('useHandleKeyUp', () => { expect(setShowPromptsPopover).toHaveBeenCalledWith(true); }); + it('triggers $ skill command for "$sk" (fast typed)', () => { + const ref = makeTextAreaRef('$sk', 3); + const { handleKeyUp, setShowSkillsPopover } = renderUseHandleKeyUp(ref); + + act(() => handleKeyUp(makeKeyEvent('k'))); + + expect(setShowSkillsPopover).toHaveBeenCalledWith(true); + }); + it('does NOT trigger for text exceeding MAX_COMMAND_TRIGGER_LENGTH', () => { const ref = makeTextAreaRef('/abcde', 6); const { handleKeyUp, setShowPromptsPopover } = renderUseHandleKeyUp(ref); @@ -337,6 +371,16 @@ describe('useHandleKeyUp', () => { expect(setShowPlusPopover).not.toHaveBeenCalled(); }); + + it('does NOT trigger $ skill command when dollarCommand toggle is disabled', () => { + mockCommandToggles.dollar = false; + const ref = makeTextAreaRef('$', 1); + const { handleKeyUp, setShowSkillsPopover } = renderUseHandleKeyUp(ref); + + act(() => handleKeyUp(makeKeyEvent('$'))); + + expect(setShowSkillsPopover).not.toHaveBeenCalled(); + }); }); describe('permission gating', () => { @@ -370,6 +414,16 @@ describe('useHandleKeyUp', () => { expect(setShowMentionPopover).toHaveBeenCalledWith(true); }); + + it('does NOT trigger $ skill command without SKILLS access', () => { + mockHasSkillsAccess.current = false; + const ref = makeTextAreaRef('$', 1); + const { handleKeyUp, setShowSkillsPopover } = renderUseHandleKeyUp(ref); + + act(() => handleKeyUp(makeKeyEvent('$'))); + + expect(setShowSkillsPopover).not.toHaveBeenCalled(); + }); }); describe('endpoint gating', () => { @@ -412,5 +466,33 @@ describe('useHandleKeyUp', () => { expect(setShowPlusPopover).toHaveBeenCalledWith(true); }); + + it('does NOT trigger $ skill command on assistants endpoint', () => { + mockEndpoint.current = 'assistants'; + const ref = makeTextAreaRef('$', 1); + const { handleKeyUp, setShowSkillsPopover } = renderUseHandleKeyUp(ref); + + act(() => handleKeyUp(makeKeyEvent('$'))); + + expect(setShowSkillsPopover).not.toHaveBeenCalledWith(true); + }); + + it('does NOT trigger $ skill command on azureAssistants endpoint', () => { + mockEndpoint.current = 'azureAssistants'; + const ref = makeTextAreaRef('$', 1); + const { handleKeyUp, setShowSkillsPopover } = renderUseHandleKeyUp(ref); + + act(() => handleKeyUp(makeKeyEvent('$'))); + + expect(setShowSkillsPopover).not.toHaveBeenCalledWith(true); + }); + + it('resets $ skills popover when endpoint switches to assistants', () => { + mockEndpoint.current = 'assistants'; + const ref = makeTextAreaRef('', 0); + const { setShowSkillsPopover } = renderUseHandleKeyUp(ref); + + expect(setShowSkillsPopover).toHaveBeenCalledWith(false); + }); }); }); diff --git a/client/src/hooks/Input/useHandleKeyUp.ts b/client/src/hooks/Input/useHandleKeyUp.ts index a2a8d2aa1d..fca2ee248a 100644 --- a/client/src/hooks/Input/useHandleKeyUp.ts +++ b/client/src/hooks/Input/useHandleKeyUp.ts @@ -61,21 +61,28 @@ const useHandleKeyUp = ({ permissionType: PermissionTypes.MULTI_CONVO, permission: Permissions.USE, }); + const hasSkillsAccess = useHasAccess({ + permissionType: PermissionTypes.SKILLS, + permission: Permissions.USE, + }); const latestMessage = useRecoilValue(store.latestMessageFamily(index)); const endpoint = useRecoilValue(store.effectiveEndpointByIndex(index)); const setShowMentionPopover = useSetRecoilState(store.showMentionPopoverFamily(index)); const setShowPlusPopover = useSetRecoilState(store.showPlusPopoverFamily(index)); const setShowPromptsPopover = useSetRecoilState(store.showPromptsPopoverFamily(index)); + const setShowSkillsPopover = useSetRecoilState(store.showSkillsPopoverFamily(index)); const atCommandEnabled = useRecoilValue(store.atCommand); const plusCommandEnabled = useRecoilValue(store.plusCommand); const slashCommandEnabled = useRecoilValue(store.slashCommand); + const dollarCommandEnabled = useRecoilValue(store.dollarCommand); useEffect(() => { if (isAssistantsEndpoint(endpoint)) { setShowPlusPopover(false); + setShowSkillsPopover(false); } - }, [endpoint, setShowPlusPopover]); + }, [endpoint, setShowPlusPopover, setShowSkillsPopover]); const handleAtCommand = useCallback(() => { if (atCommandEnabled && shouldTriggerCommand(textAreaRef, '@')) { @@ -101,13 +108,23 @@ const useHandleKeyUp = ({ } }, [textAreaRef, hasPromptsAccess, setShowPromptsPopover, slashCommandEnabled]); + const handleSkillsCommand = useCallback(() => { + if (!hasSkillsAccess || !dollarCommandEnabled || isAssistantsEndpoint(endpoint)) { + return; + } + if (shouldTriggerCommand(textAreaRef, '$')) { + setShowSkillsPopover(true); + } + }, [textAreaRef, hasSkillsAccess, setShowSkillsPopover, dollarCommandEnabled, endpoint]); + const commandHandlers = useMemo( () => ({ '@': handleAtCommand, '+': handlePlusCommand, '/': handlePromptsCommand, + $: handleSkillsCommand, }), - [handleAtCommand, handlePlusCommand, handlePromptsCommand], + [handleAtCommand, handlePlusCommand, handlePromptsCommand, handleSkillsCommand], ); const handleUpArrow = useCallback( diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index eb2122fc6c..aa09d8da93 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -1518,7 +1518,9 @@ "com_ui_skills_allow_share": "Allow sharing Skills", "com_ui_skills_allow_share_public": "Allow sharing Skills publicly", "com_ui_skills_allow_use": "Allow using Skills", + "com_ui_skills_command_placeholder": "Select a Skill by name", "com_ui_skills_empty": "No skills yet", + "com_ui_skills_load_error": "Failed to load skills", "com_ui_sr_public_skill": "Public skill", "com_ui_special": "special", "com_ui_special_var_current_date": "Current Date", diff --git a/client/src/store/families.ts b/client/src/store/families.ts index 5a28ab707e..f35bebc0d6 100644 --- a/client/src/store/families.ts +++ b/client/src/store/families.ts @@ -300,6 +300,26 @@ const showPromptsPopoverFamily = atomFamily({ default: false, }); +const showSkillsPopoverFamily = atomFamily({ + key: 'showSkillsPopoverByIndex', + default: false, +}); + +/** + * Per-conversation queue of skill names the user invoked manually via the + * `$` popover for the next submission. Acts as the structured channel + * paired with the cosmetic `$skill-name ` text inserted into the textarea. + * + * Phase 1: only the writer (SkillsCommand) is wired; the submit pipeline + * does not yet read or clear this atom. The follow-up PR will read this + * at `ask()` time, attach to the payload, and reset to `[]`. Until then + * the backend continues to receive only the textual `$name` reference. + */ +const pendingManualSkillsByConvoId = atomFamily({ + key: 'pendingManualSkillsByConvoId', + default: [], +}); + const globalAudioURLFamily = atomFamily({ key: 'globalAudioURLByIndex', default: null, @@ -497,5 +517,7 @@ export default { useClearSubmissionState, useClearLatestMessages, showPromptsPopoverFamily, + showSkillsPopoverFamily, + pendingManualSkillsByConvoId, updateConversationSelector, }; diff --git a/client/src/store/settings.ts b/client/src/store/settings.ts index 84bb815019..b0e2eef254 100644 --- a/client/src/store/settings.ts +++ b/client/src/store/settings.ts @@ -50,6 +50,7 @@ const localStorageAtoms = { atCommand: atomWithLocalStorage('atCommand', true), plusCommand: atomWithLocalStorage('plusCommand', true), slashCommand: atomWithLocalStorage('slashCommand', true), + dollarCommand: atomWithLocalStorage('dollarCommand', true), // Speech settings conversationMode: atomWithLocalStorage('conversationMode', false),