mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-20 17:13:44 +00:00
🪆 feat: Compose Agent Scope and Active-State Filters in $ Popover (#12708)
* feat: compose per-agent scope and per-user active-state filters in $ popover
Stack two runtime-truth filters on top of the existing `isUserInvocable`
check so the `$` skill popover matches what will actually be available at
turn time:
- Per-agent skill scope from `agent.skills` (resolved via useChatContext
+ useAgentsMapContext), mirroring backend `scopeSkillIds` semantics:
`undefined`/`null` → no scope, `[]` → empty, non-empty → intersection.
- Per-user ownership-aware active state via `useSkillActiveState().isActive`.
The filters are composed in a pure, exported `filterSkillsForPopover`
helper so the agent-scope ∩ active ∩ invocable matrix can be unit-tested
without rendering the component. Short-circuits on cheapest check first
(agent scope → active → invocation mode).
Backend still enforces both filters at runtime; this PR is a UX mirror so
users do not see popover entries that would be filtered out by the time
the LLM turn begins.
* refactor: thread agentId into SkillsCommand as a prop
Drop the direct `useChatContext()` call inside SkillsCommand in favor of
receiving `agentId` from ChatForm. The parent already subscribes to the
conversation via its single useChatContext call, and SkillsCommand is
wrapped in React.memo — threading the id as a prop means the popover only
re-renders when agent_id actually changes instead of on every unrelated
conversation-shape mutation. Mirrors the pattern AttachFileChat already
uses for `conversation` / `agentId`.
No behavior change; filter semantics and test coverage are identical.
* fix: fail closed when agent skill scope cannot be resolved
Previously, if `conversation.agent_id` was set but the agents map had no
entry for it (hydration pending, query failure, or missing VIEW access),
the popover treated the scope as `undefined` and showed the full ACL
catalog. That leaks options the backend will reject at turn time, the
opposite of what this phase is meant to do.
Distinguish the unresolved cases ("map undefined" and "agent not in map")
from the intentionally-unconfigured case ("agent exists, no `skills`
field") and return `[]` for the former, preserving the backend semantics
of `scopeSkillIds` only for the latter. Adds two tests covering both
fail-closed branches.
* fix: surface agent.skills in list projection and treat ephemeral ids as unscoped
Two holes flagged on the earlier fail-closed commit:
1. The list-agents projection in `getListAgentsByAccess` omitted the
`skills` field, so `agentsMap[agentId].skills` was always undefined
and the popover fell back to the full ACL catalog for every scoped
agent — the opposite of this phase's intent. Add `skills: 1` to the
projection and lock it in with a new test.
2. Conversations can carry ephemeral agent ids (e.g. `ephemeral` after
switching off the agents endpoint) that intentionally don't live in
the agents map. The prior fail-closed branch blanked the popover in
those cases. Treat anything that doesn't start with `agent_` as
unscoped via the existing `isEphemeralAgent` helper so the popover
shows the full ACL-visible catalog, matching how no-agent convos
already behave.
Frontend: 18 tests pass (adds one ephemeral-id case).
Backend: 10 getListAgentsByAccess tests pass (adds one projection case).
* refactor: pass through hydration race, only fail closed when map is authoritative
Split the two "cannot resolve scope" cases that were previously collapsed
onto the same fail-closed branch:
- `agentsMap === undefined` means the agents list query has not settled
yet. Return `undefined` (full catalog). The map typically hydrates well
before the first `$` open, and the backend still scopes at turn time —
blanking the popover during a sub-second race produces worse UX with
no security benefit.
- `agentsMap` is populated but the agent is absent means the agent was
deleted or the user's VIEW access was revoked mid-session. That is
authoritative missing state, so keep the fail-closed behavior — the
full catalog would be misleading.
Updates the associated test case to assert the hydration-race path now
shows the catalog, and rewrites the in-code comment to distinguish the
two branches.
* refactor: drop `as string` in agent scope memo by tightening the guard
`isEphemeralAgent` returns true for null/undefined so the original code
was runtime-safe, but its signature returns plain `boolean` rather than
a type predicate, so TypeScript never narrowed `agentId` to `string` and
the subsequent map lookup required an `as string` assertion. Split the
guard into `!agentId || isEphemeralAgent(agentId)` so the narrowing falls
out naturally and the assertion can be removed.
No behavior change; 18 tests pass.
This commit is contained in:
parent
9225a279eb
commit
3f1bde4831
5 changed files with 433 additions and 30 deletions
|
|
@ -255,7 +255,12 @@ const ChatForm = memo(function ChatForm({
|
|||
textAreaRef={textAreaRef}
|
||||
/>
|
||||
<PromptsCommand index={index} textAreaRef={textAreaRef} submitPrompt={submitPrompt} />
|
||||
<SkillsCommand index={index} textAreaRef={textAreaRef} conversationId={conversationId} />
|
||||
<SkillsCommand
|
||||
index={index}
|
||||
textAreaRef={textAreaRef}
|
||||
conversationId={conversationId}
|
||||
agentId={conversation?.agent_id}
|
||||
/>
|
||||
<div
|
||||
onClick={handleContainerClick}
|
||||
className={cn(
|
||||
|
|
|
|||
|
|
@ -7,11 +7,13 @@ import { useRecoilValue, useSetRecoilState } from 'recoil';
|
|||
import type { TSkillSummary } from 'librechat-data-provider';
|
||||
import type { MentionOption } from '~/common';
|
||||
import useInitPopoverInput from '~/hooks/Input/useInitPopoverInput';
|
||||
import { useLocalize, useSkillActiveState } from '~/hooks';
|
||||
import { useAgentsMapContext } from '~/Providers';
|
||||
import { useSkillsInfiniteQuery } from '~/data-provider';
|
||||
import { isEphemeralAgent } from '~/common';
|
||||
import { ephemeralAgentByConvoId } from '~/store';
|
||||
import { removeCharIfLast } from '~/utils';
|
||||
import MentionItem from './MentionItem';
|
||||
import { useLocalize } from '~/hooks';
|
||||
import store from '~/store';
|
||||
|
||||
const commandChar = '$';
|
||||
|
|
@ -32,14 +34,61 @@ export function isUserInvocable(skill: TSkillSummary): boolean {
|
|||
return mode === InvocationMode.manual;
|
||||
}
|
||||
|
||||
/**
|
||||
* Filters the skills list down to what should appear in the `$` popover.
|
||||
* Composes three rules, short-circuiting on the cheapest check first:
|
||||
*
|
||||
* 1. Agent scope — mirrors backend `scopeSkillIds` semantics:
|
||||
* - `null` / `undefined` → no scope filter (ephemeral convo, or agent
|
||||
* without a `skills` field configured).
|
||||
* - `[]` → explicit opt-out, nothing passes.
|
||||
* - non-empty → intersection with the agent's configured skill ids.
|
||||
* 2. Active state — per-user ownership-aware toggle.
|
||||
* 3. Invocation mode — `manual` / `both` / undefined are visible; `auto`
|
||||
* is model-only and hidden.
|
||||
*
|
||||
* Pure function; exported so tests can exercise the filter in isolation
|
||||
* without rendering the component.
|
||||
*/
|
||||
export function filterSkillsForPopover(
|
||||
skills: TSkillSummary[],
|
||||
ctx: {
|
||||
agentSkillIds: string[] | null | undefined;
|
||||
isActive: (skill: Pick<TSkillSummary, '_id' | 'author'>) => boolean;
|
||||
},
|
||||
): TSkillSummary[] {
|
||||
const { agentSkillIds, isActive } = ctx;
|
||||
if (agentSkillIds != null && agentSkillIds.length === 0) {
|
||||
return [];
|
||||
}
|
||||
const agentSet =
|
||||
agentSkillIds != null && agentSkillIds.length > 0 ? new Set(agentSkillIds) : null;
|
||||
const result: TSkillSummary[] = [];
|
||||
for (const skill of skills) {
|
||||
if (agentSet && !agentSet.has(skill._id)) {
|
||||
continue;
|
||||
}
|
||||
if (!isActive(skill)) {
|
||||
continue;
|
||||
}
|
||||
if (!isUserInvocable(skill)) {
|
||||
continue;
|
||||
}
|
||||
result.push(skill);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
function SkillsCommandContent({
|
||||
index,
|
||||
textAreaRef,
|
||||
conversationId,
|
||||
agentId,
|
||||
}: {
|
||||
index: number;
|
||||
textAreaRef: React.MutableRefObject<HTMLTextAreaElement | null>;
|
||||
conversationId: string;
|
||||
agentId?: string | null;
|
||||
}) {
|
||||
const localize = useLocalize();
|
||||
const setShowSkillsPopover = useSetRecoilState(store.showSkillsPopoverFamily(index));
|
||||
|
|
@ -48,6 +97,36 @@ function SkillsCommandContent({
|
|||
store.pendingManualSkillsByConvoId(conversationId),
|
||||
);
|
||||
|
||||
const agentsMap = useAgentsMapContext();
|
||||
const { isActive } = useSkillActiveState();
|
||||
|
||||
/* Resolve the per-agent skill scope. Mirrors backend `scopeSkillIds` for
|
||||
the happy path: no `skills` field → no scope, `[]` → opt-out, non-empty
|
||||
→ intersection. Ephemeral agent ids (null/undefined/placeholder strings
|
||||
that don't begin with `agent_`) are unscoped — they correspond to
|
||||
conversations without a persisted agent and are intentionally absent
|
||||
from the agents map. While the map is still hydrating we pass through
|
||||
(undefined → full catalog): the backend enforces scope at turn time, so
|
||||
there's no security benefit to flashing an empty popover, and the map
|
||||
typically lands well before the first open. Once the map is authoritative
|
||||
but the agent isn't in it (deleted, or VIEW revoked mid-session), we fail
|
||||
closed — scope is unresolvable and the full catalog would be misleading.
|
||||
`agentId` is threaded in as a prop so this component stays memoizable
|
||||
and skips re-renders on unrelated conversation-shape changes. */
|
||||
const agentSkillIds = useMemo<string[] | null | undefined>(() => {
|
||||
if (!agentId || isEphemeralAgent(agentId)) {
|
||||
return undefined;
|
||||
}
|
||||
if (!agentsMap) {
|
||||
return undefined;
|
||||
}
|
||||
const agent = agentsMap[agentId];
|
||||
if (!agent) {
|
||||
return [];
|
||||
}
|
||||
return agent.skills;
|
||||
}, [agentId, agentsMap]);
|
||||
|
||||
const { data, isLoading, isError, fetchNextPage, hasNextPage, isFetchingNextPage } =
|
||||
useSkillsInfiniteQuery({ limit: 50 });
|
||||
|
||||
|
|
@ -77,21 +156,25 @@ function SkillsCommandContent({
|
|||
if (!data?.pages) {
|
||||
return [];
|
||||
}
|
||||
return data.pages.reduce<MentionOption[]>((acc, page) => {
|
||||
const allSkills: TSkillSummary[] = [];
|
||||
for (const page of data.pages) {
|
||||
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,
|
||||
});
|
||||
}
|
||||
allSkills.push(skill);
|
||||
}
|
||||
return acc;
|
||||
}, []);
|
||||
}, [data?.pages]);
|
||||
}
|
||||
const filtered = filterSkillsForPopover(allSkills, { agentSkillIds, isActive });
|
||||
const options: MentionOption[] = [];
|
||||
for (const skill of filtered) {
|
||||
options.push({
|
||||
label: skill.displayTitle ?? skill.name,
|
||||
value: skill.name,
|
||||
description: skill.description,
|
||||
type: 'skill',
|
||||
icon: skillIcon,
|
||||
});
|
||||
}
|
||||
return options;
|
||||
}, [data?.pages, agentSkillIds, isActive]);
|
||||
|
||||
const [activeIndex, setActiveIndex] = useState(0);
|
||||
const timeoutRef = useRef<NodeJS.Timeout | null>(null);
|
||||
|
|
@ -314,17 +397,24 @@ const SkillsCommand = memo(function SkillsCommand({
|
|||
index,
|
||||
textAreaRef,
|
||||
conversationId,
|
||||
agentId,
|
||||
}: {
|
||||
index: number;
|
||||
textAreaRef: React.MutableRefObject<HTMLTextAreaElement | null>;
|
||||
conversationId: string;
|
||||
agentId?: string | null;
|
||||
}) {
|
||||
const show = useRecoilValue(store.showSkillsPopoverFamily(index));
|
||||
if (!show) {
|
||||
return null;
|
||||
}
|
||||
return (
|
||||
<SkillsCommandContent index={index} textAreaRef={textAreaRef} conversationId={conversationId} />
|
||||
<SkillsCommandContent
|
||||
index={index}
|
||||
textAreaRef={textAreaRef}
|
||||
conversationId={conversationId}
|
||||
agentId={agentId}
|
||||
/>
|
||||
);
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -4,11 +4,18 @@
|
|||
* 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.
|
||||
*
|
||||
* Also covers the Phase 2 filter composition: per-agent skill scope
|
||||
* intersects with the ACL catalog, and per-user active-state toggles
|
||||
* hide inactive entries from the popover (backend still enforces both
|
||||
* at runtime regardless).
|
||||
*/
|
||||
import React from 'react';
|
||||
import { act } from '@testing-library/react';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
import { render, screen } from '@testing-library/react';
|
||||
import { InvocationMode } from 'librechat-data-provider';
|
||||
import type { TSkillSummary } from 'librechat-data-provider';
|
||||
|
||||
const CONVO_ID = 'convo-1';
|
||||
|
||||
|
|
@ -58,8 +65,20 @@ jest.mock('~/data-provider', () => ({
|
|||
useSkillsInfiniteQuery: () => mockUseSkillsInfiniteQuery(),
|
||||
}));
|
||||
|
||||
/* Phase 2: the popover reads agent skill config via useAgentsMapContext
|
||||
and the agent id is threaded down as a prop from ChatForm (so the
|
||||
component stays memoizable across unrelated convo-shape changes). The
|
||||
test harness swaps the agents map in so cases can configure ephemeral
|
||||
vs. agent-scoped behavior without standing up a real provider. */
|
||||
const mockUseAgentsMapContext = jest.fn();
|
||||
jest.mock('~/Providers', () => ({
|
||||
useAgentsMapContext: () => mockUseAgentsMapContext(),
|
||||
}));
|
||||
|
||||
const mockIsActive = jest.fn();
|
||||
jest.mock('~/hooks', () => ({
|
||||
useLocalize: () => (key: string) => key,
|
||||
useSkillActiveState: () => ({ isActive: mockIsActive }),
|
||||
}));
|
||||
|
||||
jest.mock('@librechat/client', () => {
|
||||
|
|
@ -95,7 +114,7 @@ jest.mock('react-virtualized', () => ({
|
|||
},
|
||||
}));
|
||||
|
||||
import SkillsCommand from '../SkillsCommand';
|
||||
import SkillsCommand, { filterSkillsForPopover } from '../SkillsCommand';
|
||||
|
||||
const makeTextarea = (initial = '$') => {
|
||||
const textarea = document.createElement('textarea');
|
||||
|
|
@ -104,23 +123,37 @@ const makeTextarea = (initial = '$') => {
|
|||
return { current: textarea } as React.MutableRefObject<HTMLTextAreaElement | null>;
|
||||
};
|
||||
|
||||
const makeSkill = (overrides: Partial<TSkillSummary>): TSkillSummary => ({
|
||||
_id: '1',
|
||||
name: 'brand-guidelines',
|
||||
displayTitle: 'Brand Guidelines',
|
||||
description: 'Apply brand styling',
|
||||
author: 'u',
|
||||
authorName: 'U',
|
||||
version: 1,
|
||||
source: 'inline',
|
||||
fileCount: 0,
|
||||
createdAt: '',
|
||||
updatedAt: '',
|
||||
...overrides,
|
||||
});
|
||||
|
||||
const skillsResponse = {
|
||||
pages: [
|
||||
{
|
||||
skills: [makeSkill({})],
|
||||
has_more: false,
|
||||
after: null,
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
const twoSkillsResponse = {
|
||||
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: '',
|
||||
},
|
||||
makeSkill({ _id: '1', name: 'brand-guidelines', displayTitle: 'Brand Guidelines' }),
|
||||
makeSkill({ _id: '2', name: 'style-guide', displayTitle: 'Style Guide' }),
|
||||
],
|
||||
has_more: false,
|
||||
after: null,
|
||||
|
|
@ -140,6 +173,11 @@ beforeEach(() => {
|
|||
hasNextPage: false,
|
||||
isFetchingNextPage: false,
|
||||
});
|
||||
/* Defaults: empty agents map and every skill active. Individual tests
|
||||
override these and pass `agentId` as a prop to exercise the Phase 2
|
||||
filter composition. */
|
||||
mockUseAgentsMapContext.mockReturnValue({});
|
||||
mockIsActive.mockReturnValue(true);
|
||||
});
|
||||
|
||||
describe('SkillsCommand', () => {
|
||||
|
|
@ -184,4 +222,244 @@ describe('SkillsCommand', () => {
|
|||
/* Popover dismisses on selection. */
|
||||
expect(mockSetShowSkillsPopover).toHaveBeenCalledWith(false);
|
||||
});
|
||||
|
||||
it('narrows the list to the agent-configured scope when agent.skills is set', async () => {
|
||||
mockUseSkillsInfiniteQuery.mockReturnValue({
|
||||
data: twoSkillsResponse,
|
||||
isLoading: false,
|
||||
isError: false,
|
||||
fetchNextPage: jest.fn(),
|
||||
hasNextPage: false,
|
||||
isFetchingNextPage: false,
|
||||
});
|
||||
mockUseAgentsMapContext.mockReturnValue({
|
||||
agent_1: { id: 'agent_1', skills: ['2'] },
|
||||
});
|
||||
|
||||
const textAreaRef = makeTextarea('$');
|
||||
render(
|
||||
<SkillsCommand
|
||||
index={0}
|
||||
textAreaRef={textAreaRef}
|
||||
conversationId={CONVO_ID}
|
||||
agentId="agent_1"
|
||||
/>,
|
||||
);
|
||||
|
||||
/* Only the skill whose _id is in agent.skills should appear. */
|
||||
expect(screen.queryByRole('button', { name: /Brand Guidelines/i })).toBeNull();
|
||||
expect(await screen.findByRole('button', { name: /Style Guide/i })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('shows nothing when the agent has an empty skills array (explicit opt-out)', () => {
|
||||
mockUseSkillsInfiniteQuery.mockReturnValue({
|
||||
data: twoSkillsResponse,
|
||||
isLoading: false,
|
||||
isError: false,
|
||||
fetchNextPage: jest.fn(),
|
||||
hasNextPage: false,
|
||||
isFetchingNextPage: false,
|
||||
});
|
||||
mockUseAgentsMapContext.mockReturnValue({
|
||||
agent_1: { id: 'agent_1', skills: [] },
|
||||
});
|
||||
|
||||
const textAreaRef = makeTextarea('$');
|
||||
render(
|
||||
<SkillsCommand
|
||||
index={0}
|
||||
textAreaRef={textAreaRef}
|
||||
conversationId={CONVO_ID}
|
||||
agentId="agent_1"
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.queryByRole('button', { name: /Brand Guidelines/i })).toBeNull();
|
||||
expect(screen.queryByRole('button', { name: /Style Guide/i })).toBeNull();
|
||||
});
|
||||
|
||||
it('shows the full ACL catalog when the agent has no skills field configured', async () => {
|
||||
mockUseSkillsInfiniteQuery.mockReturnValue({
|
||||
data: twoSkillsResponse,
|
||||
isLoading: false,
|
||||
isError: false,
|
||||
fetchNextPage: jest.fn(),
|
||||
hasNextPage: false,
|
||||
isFetchingNextPage: false,
|
||||
});
|
||||
mockUseAgentsMapContext.mockReturnValue({
|
||||
agent_1: { id: 'agent_1' },
|
||||
});
|
||||
|
||||
const textAreaRef = makeTextarea('$');
|
||||
render(
|
||||
<SkillsCommand
|
||||
index={0}
|
||||
textAreaRef={textAreaRef}
|
||||
conversationId={CONVO_ID}
|
||||
agentId="agent_1"
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(await screen.findByRole('button', { name: /Brand Guidelines/i })).toBeInTheDocument();
|
||||
expect(await screen.findByRole('button', { name: /Style Guide/i })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('treats an ephemeral agent id as unscoped and shows the full ACL catalog', async () => {
|
||||
mockUseSkillsInfiniteQuery.mockReturnValue({
|
||||
data: twoSkillsResponse,
|
||||
isLoading: false,
|
||||
isError: false,
|
||||
fetchNextPage: jest.fn(),
|
||||
hasNextPage: false,
|
||||
isFetchingNextPage: false,
|
||||
});
|
||||
mockUseAgentsMapContext.mockReturnValue({});
|
||||
|
||||
const textAreaRef = makeTextarea('$');
|
||||
render(
|
||||
<SkillsCommand
|
||||
index={0}
|
||||
textAreaRef={textAreaRef}
|
||||
conversationId={CONVO_ID}
|
||||
agentId="ephemeral"
|
||||
/>,
|
||||
);
|
||||
|
||||
/* `ephemeral` doesn't start with `agent_`, so it's an ephemeral id;
|
||||
scope filter should be skipped and the full catalog displayed. */
|
||||
expect(await screen.findByRole('button', { name: /Brand Guidelines/i })).toBeInTheDocument();
|
||||
expect(await screen.findByRole('button', { name: /Style Guide/i })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('shows the full ACL catalog while the agents map is hydrating (backend still gates the turn)', async () => {
|
||||
mockUseSkillsInfiniteQuery.mockReturnValue({
|
||||
data: twoSkillsResponse,
|
||||
isLoading: false,
|
||||
isError: false,
|
||||
fetchNextPage: jest.fn(),
|
||||
hasNextPage: false,
|
||||
isFetchingNextPage: false,
|
||||
});
|
||||
mockUseAgentsMapContext.mockReturnValue(undefined);
|
||||
|
||||
const textAreaRef = makeTextarea('$');
|
||||
render(
|
||||
<SkillsCommand
|
||||
index={0}
|
||||
textAreaRef={textAreaRef}
|
||||
conversationId={CONVO_ID}
|
||||
agentId="agent_1"
|
||||
/>,
|
||||
);
|
||||
|
||||
/* Hydration race: map not yet loaded. Pass through to full catalog —
|
||||
the backend scopes at turn time and blanking the popover during
|
||||
sub-second hydration is worse UX for no security benefit. */
|
||||
expect(await screen.findByRole('button', { name: /Brand Guidelines/i })).toBeInTheDocument();
|
||||
expect(await screen.findByRole('button', { name: /Style Guide/i })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('fails closed when the agent id is set but missing from the agents map', () => {
|
||||
mockUseSkillsInfiniteQuery.mockReturnValue({
|
||||
data: twoSkillsResponse,
|
||||
isLoading: false,
|
||||
isError: false,
|
||||
fetchNextPage: jest.fn(),
|
||||
hasNextPage: false,
|
||||
isFetchingNextPage: false,
|
||||
});
|
||||
mockUseAgentsMapContext.mockReturnValue({});
|
||||
|
||||
const textAreaRef = makeTextarea('$');
|
||||
render(
|
||||
<SkillsCommand
|
||||
index={0}
|
||||
textAreaRef={textAreaRef}
|
||||
conversationId={CONVO_ID}
|
||||
agentId="agent_1"
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.queryByRole('button', { name: /Brand Guidelines/i })).toBeNull();
|
||||
expect(screen.queryByRole('button', { name: /Style Guide/i })).toBeNull();
|
||||
});
|
||||
|
||||
it('hides inactive skills from the popover', () => {
|
||||
mockUseSkillsInfiniteQuery.mockReturnValue({
|
||||
data: twoSkillsResponse,
|
||||
isLoading: false,
|
||||
isError: false,
|
||||
fetchNextPage: jest.fn(),
|
||||
hasNextPage: false,
|
||||
isFetchingNextPage: false,
|
||||
});
|
||||
mockIsActive.mockImplementation((skill: { _id: string }) => skill._id !== '1');
|
||||
|
||||
const textAreaRef = makeTextarea('$');
|
||||
render(<SkillsCommand index={0} textAreaRef={textAreaRef} conversationId={CONVO_ID} />);
|
||||
|
||||
expect(screen.queryByRole('button', { name: /Brand Guidelines/i })).toBeNull();
|
||||
expect(screen.getByRole('button', { name: /Style Guide/i })).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
describe('filterSkillsForPopover', () => {
|
||||
const active = () => true;
|
||||
const inactive = () => false;
|
||||
const s1 = makeSkill({ _id: '1', name: 'a' });
|
||||
const s2 = makeSkill({ _id: '2', name: 'b' });
|
||||
const s3 = makeSkill({ _id: '3', name: 'c', invocationMode: InvocationMode.auto });
|
||||
|
||||
it('passes everything through when agentSkillIds is undefined', () => {
|
||||
const out = filterSkillsForPopover([s1, s2], { agentSkillIds: undefined, isActive: active });
|
||||
expect(out.map((s) => s._id)).toEqual(['1', '2']);
|
||||
});
|
||||
|
||||
it('passes everything through when agentSkillIds is null', () => {
|
||||
const out = filterSkillsForPopover([s1, s2], { agentSkillIds: null, isActive: active });
|
||||
expect(out.map((s) => s._id)).toEqual(['1', '2']);
|
||||
});
|
||||
|
||||
it('returns empty when agentSkillIds is []', () => {
|
||||
const out = filterSkillsForPopover([s1, s2], { agentSkillIds: [], isActive: active });
|
||||
expect(out).toEqual([]);
|
||||
});
|
||||
|
||||
it('intersects with a non-empty agentSkillIds', () => {
|
||||
const out = filterSkillsForPopover([s1, s2], { agentSkillIds: ['2'], isActive: active });
|
||||
expect(out.map((s) => s._id)).toEqual(['2']);
|
||||
});
|
||||
|
||||
it('excludes inactive skills', () => {
|
||||
const isActive = (skill: { _id: string }) => skill._id !== '1';
|
||||
const out = filterSkillsForPopover([s1, s2], { agentSkillIds: null, isActive });
|
||||
expect(out.map((s) => s._id)).toEqual(['2']);
|
||||
});
|
||||
|
||||
it('excludes auto-only skills via isUserInvocable', () => {
|
||||
const out = filterSkillsForPopover([s1, s3], { agentSkillIds: null, isActive: active });
|
||||
expect(out.map((s) => s._id)).toEqual(['1']);
|
||||
});
|
||||
|
||||
it('still empty when agent scope is [] even if everything is active and invocable', () => {
|
||||
const out = filterSkillsForPopover([s1, s2, s3], { agentSkillIds: [], isActive: active });
|
||||
expect(out).toEqual([]);
|
||||
});
|
||||
|
||||
it('layers all three filters (agent scope ∩ active ∩ invocable)', () => {
|
||||
const isActive = (skill: { _id: string }) => skill._id !== '2';
|
||||
const out = filterSkillsForPopover([s1, s2, s3], {
|
||||
agentSkillIds: ['1', '2', '3'],
|
||||
isActive,
|
||||
});
|
||||
/* s1 passes (active, manual-by-default, scoped), s2 drops (inactive),
|
||||
s3 drops (auto-only invocation mode). */
|
||||
expect(out.map((s) => s._id)).toEqual(['1']);
|
||||
});
|
||||
|
||||
it('drops everything when isActive returns false for all inputs', () => {
|
||||
const out = filterSkillsForPopover([s1, s2], { agentSkillIds: null, isActive: inactive });
|
||||
expect(out).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -3334,6 +3334,33 @@ describe('Support Contact Field', () => {
|
|||
expect(result.data[0].name).toBe('Agent A1');
|
||||
});
|
||||
|
||||
test('should include the skills field in the list projection', async () => {
|
||||
// Frontend popover scoping relies on `agent.skills` being present in
|
||||
// list results so it can narrow the `$` catalog without refetching the
|
||||
// full agent document. Locks in that projection contract.
|
||||
const targetSkillIds = [
|
||||
new mongoose.Types.ObjectId().toString(),
|
||||
new mongoose.Types.ObjectId().toString(),
|
||||
];
|
||||
const scopedAgent = await createAgent({
|
||||
id: `agent_${uuidv4().slice(0, 12)}`,
|
||||
name: 'Scoped Agent',
|
||||
description: 'Agent with configured skill scope',
|
||||
provider: 'openai',
|
||||
model: 'gpt-4',
|
||||
author: userA,
|
||||
skills: targetSkillIds,
|
||||
});
|
||||
|
||||
const result = await getListAgentsByAccess({
|
||||
accessibleIds: [scopedAgent._id] as mongoose.Types.ObjectId[],
|
||||
otherParams: {},
|
||||
});
|
||||
|
||||
expect(result.data).toHaveLength(1);
|
||||
expect(result.data[0].skills).toEqual(targetSkillIds);
|
||||
});
|
||||
|
||||
test('should return multiple accessible agents when provided', async () => {
|
||||
// Give User B access to two of User A's agents
|
||||
const accessibleIds = [agentA1._id, agentA3._id] as mongoose.Types.ObjectId[];
|
||||
|
|
|
|||
|
|
@ -711,6 +711,9 @@ export function createAgentMethods(mongoose: typeof import('mongoose'), deps: Ag
|
|||
category: 1,
|
||||
support_contact: 1,
|
||||
is_promoted: 1,
|
||||
/* Needed so the client can scope the `$` skill popover to each agent's
|
||||
configured catalog without refetching the full agent document. */
|
||||
skills: 1,
|
||||
}).sort({ updatedAt: -1, _id: 1 });
|
||||
|
||||
if (isPaginated && normalizedLimit) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue