fix: sync skills_enabled with selection edits and hydrate agent file entries

skills_enabled is the master opt-in for the skill allowlist, and an empty
allowlist with the flag on means the full accessible catalog. Selection
edits now sync the flag on empty/non-empty transitions via a shared
skillsEnabledTransition helper: picking the first skill enables it so the
choice takes effect on save, and removing the last one disables it so the
agent doesn't silently escalate to every skill. Mid-selection edits leave
the flag alone, preserving the Advanced kill switch's
disable-without-clearing behavior.

Agents loaded from the API carry only tool_resources.*.file_ids; the
client-only context/knowledge/code file entry arrays were read directly,
so existing attachments rendered as empty and could not be removed. A new
useAgentFileEntries hook restores the legacy derivation (agent files query
merged into the file map via processAgentOption) and now feeds AgentConfig,
the item dialog, and the selected-items pipeline.
This commit is contained in:
Marco Beretta 2026-07-03 12:27:59 +02:00
parent 05ac524fd9
commit 0e262eda5b
No known key found for this signature in database
GPG key ID: D918033D8E74CC11
8 changed files with 152 additions and 52 deletions

View file

@ -1,10 +1,11 @@
import { Input } from '@librechat/client';
import { Controller, useWatch, useFormContext } from 'react-hook-form';
import { EModelEndpoint, getEndpointField } from 'librechat-data-provider';
import type { AgentForm, ExtendedFile, IconComponentTypes } from '~/common';
import { useLocalize, useAgentCapabilities } from '~/hooks';
import type { AgentForm, IconComponentTypes } from '~/common';
import AgentCategorySelector from './AgentCategorySelector';
import { useLocalize, useAgentCapabilities } from '~/hooks';
import { validateEmail, getIconKey, cn } from '~/utils';
import { useAgentFileEntries } from './Tools/hooks';
import { useAgentPanelContext } from '~/Providers';
import ToolsSection from './Tools/ToolsSection';
import { icons } from '~/hooks/Endpoint/Icons';
@ -29,7 +30,7 @@ export default function AgentConfig() {
const model = useWatch({ control, name: 'model' });
const agent = useWatch({ control, name: 'agent' });
const agent_id = useWatch({ control, name: 'id' });
const contextFiles = (agent?.context_files ?? []) as Array<[string, ExtendedFile]>;
const { contextFiles } = useAgentFileEntries();
const providerValue = typeof provider === 'string' ? provider : provider?.value;
let Icon: IconComponentTypes | null | undefined;

View file

@ -1,12 +1,10 @@
import { useMemo } from 'react';
import { useFormContext, useWatch } from 'react-hook-form';
import type { ExtendedFile, AgentForm } from '~/common';
import type { AgentItem } from '../items/types';
import BuiltinSection from './sections/BuiltinSection';
import ToolSection from './sections/ToolSection';
import SkillSection from './sections/SkillSection';
import McpSection from './sections/McpSection';
import ActionSection from './sections/ActionSection';
import SkillSection from './sections/SkillSection';
import ToolSection from './sections/ToolSection';
import McpSection from './sections/McpSection';
import { useAgentFileEntries } from '../hooks';
interface Props {
item: AgentItem;
@ -15,21 +13,7 @@ interface Props {
}
export default function ItemDialogBody({ item, agentId, onClose }: Props) {
const { control } = useFormContext<AgentForm>();
const agent = useWatch({ control, name: 'agent' });
const contextFiles = useMemo(
() => (agent?.context_files ?? []) as Array<[string, ExtendedFile]>,
[agent?.context_files],
);
const knowledgeFiles = useMemo(
() => (agent?.knowledge_files ?? []) as Array<[string, ExtendedFile]>,
[agent?.knowledge_files],
);
const codeFiles = useMemo(
() => (agent?.code_files ?? []) as Array<[string, ExtendedFile]>,
[agent?.code_files],
);
const { contextFiles, knowledgeFiles, codeFiles } = useAgentFileEntries();
if (item.kind === 'builtin') {
return (

View file

@ -1,13 +1,17 @@
import '@testing-library/jest-dom/extend-expect';
import { render, screen } from '@testing-library/react';
import ItemDialogBody from '../ItemDialogBody';
import type { AgentItem } from '../../items/types';
import ItemDialogBody from '../ItemDialogBody';
jest.mock('react-hook-form', () => ({
useFormContext: () => ({ control: {}, getValues: () => undefined, setValue: jest.fn() }),
useWatch: () => undefined,
}));
jest.mock('../../hooks', () => ({
useAgentFileEntries: () => ({ contextFiles: [], knowledgeFiles: [], codeFiles: [] }),
}));
jest.mock('../sections/BuiltinSection', () => ({
__esModule: true,
default: ({ builtinId }: { builtinId: string }) => (

View file

@ -18,6 +18,7 @@ import type { AgentItem } from './items/types';
import type { AgentForm } from '~/common';
import { useLocalize, useHasAccess, useAuthContext, useToolFavorites } from '~/hooks';
import { CreateSkillDialog } from '~/components/Skills/dialogs';
import { skillsEnabledTransition } from './items/mutations';
import MarketplaceCatalog from './MarketplaceCatalog';
import { useListSkillsQuery } from '~/data-provider';
import { CategoryIcon } from '~/components/Prompts';
@ -108,29 +109,37 @@ export default function SkillsDialog({ open, onOpenChange, agentId }: SkillsDial
[catalog, search, category, view, favoriteKeys],
);
const applySkillsSelection = useCallback(
(next: string[]) => {
const current = (getValues('skills') ?? []) as string[];
setValue('skills', next, { shouldDirty: true });
const flag = skillsEnabledTransition(current, next, getValues('skills_enabled'));
if (flag !== undefined) {
setValue('skills_enabled', flag, { shouldDirty: true });
}
},
[getValues, setValue],
);
const handleSkillCreated = useCallback(
(skill: TSkill) => {
const current = (getValues('skills') ?? []) as string[];
setValue('skills', Array.from(new Set([...current, skill._id])), { shouldDirty: true });
applySkillsSelection(Array.from(new Set([...current, skill._id])));
setView('mine');
},
[getValues, setValue],
[getValues, applySkillsSelection],
);
const handleToggle = useCallback(
(item: AgentItem) => {
const current = (getValues('skills') ?? []) as string[];
if (selectedIds.has(itemKey(item))) {
setValue(
'skills',
current.filter((id) => id !== item.id),
{ shouldDirty: true },
);
applySkillsSelection(current.filter((id) => id !== item.id));
return;
}
setValue('skills', Array.from(new Set([...current, item.id])), { shouldDirty: true });
applySkillsSelection(Array.from(new Set([...current, item.id])));
},
[getValues, setValue, selectedIds],
[getValues, applySkillsSelection, selectedIds],
);
const viewOptions = useMemo(

View file

@ -7,10 +7,10 @@ import type { TPlugin } from 'librechat-data-provider';
import type { AgentItem } from './items/types';
import type { AgentForm } from '~/common';
import { useAgentItems, useResolvedSkills, useUninstallToolCredentials } from './hooks';
import { computeToggleAction, skillsEnabledTransition } from './items/mutations';
import { useListSkillsQuery, useDeleteAgentAction } from '~/data-provider';
import { useRemoveMCPTool, useVisibleTools } from '~/hooks/MCP';
import ToolsMarketplaceDialog from './ToolsMarketplaceDialog';
import { computeToggleAction } from './items/mutations';
import { useLocalize, useHasAccess } from '~/hooks';
import { useAgentPanelContext } from '~/Providers';
import ItemDialog from './ItemDialog/ItemDialog';
@ -92,11 +92,12 @@ export default function ToolsSection({ agentId }: Props) {
}
case 'skill-remove': {
const current = (getValues('skills') ?? []) as string[];
setValue(
'skills',
current.filter((s) => s !== patch.id),
{ shouldDirty: true },
);
const next = current.filter((s) => s !== patch.id);
setValue('skills', next, { shouldDirty: true });
const flag = skillsEnabledTransition(current, next, getValues('skills_enabled'));
if (flag !== undefined) {
setValue('skills_enabled', flag, { shouldDirty: true });
}
break;
}
case 'mcp-remove':

View file

@ -13,14 +13,15 @@ import {
AgentCapabilities,
} from 'librechat-data-provider';
import type { TSkillSummary } from 'librechat-data-provider';
import type { AgentForm, ExtendedFile } from '~/common';
import type { AgentItem } from './items/types';
import type { AgentForm } from '~/common';
import { useVerifyAgentToolAuth, useGetAgentFiles } from '~/data-provider';
import { useLocalize, useHasAccess, useHasMemoryAccess } from '~/hooks';
import { useVerifyAgentToolAuth } from '~/data-provider';
import { useFileMapContext, useAgentPanelContext } from '~/Providers';
import { deriveSelectedItems } from './items/selectors';
import { useAuthContext } from '~/hooks/AuthContext';
import { useAgentPanelContext } from '~/Providers';
import { buildCatalog } from './items/catalog';
import { processAgentOption } from '~/utils';
/**
* Maps builtin capability ids to whether they still need setup (USER_PROVIDED auth
@ -75,6 +76,55 @@ export function useShowMemory(): boolean {
}, [agentsConfig, hasMemoryAccess, user]);
}
export interface AgentFileEntries {
contextFiles: Array<[string, ExtendedFile]>;
knowledgeFiles: Array<[string, ExtendedFile]>;
codeFiles: Array<[string, ExtendedFile]>;
}
const NO_FILES: Array<[string, ExtendedFile]> = [];
/**
* File entries for the agent's builtin file tools (File Context, File Search,
* Code Interpreter). Agents loaded from the API carry only
* `tool_resources.*.file_ids` the client-side entry arrays exist on the form
* `agent` object only after an in-session upload. Reading them directly would
* show an existing agent's attachments as empty, so this derives the missing
* arrays by merging the agent files query into the file map, exactly like the
* legacy `AgentConfig` hydration. Must be rendered inside the agent form's
* `FormProvider` and the file map context.
*/
export function useAgentFileEntries(): AgentFileEntries {
const { control } = useFormContext<AgentForm>();
const agent = useWatch({ control, name: 'agent' });
const agentId = useWatch({ control, name: 'id' });
const fileMap = useFileMapContext();
const { data: agentFiles = [] } = useGetAgentFiles(agentId);
return useMemo(() => {
if (agent == null || agent.id !== agentId) {
return { contextFiles: NO_FILES, knowledgeFiles: NO_FILES, codeFiles: NO_FILES };
}
const needsHydration =
agent.context_files == null || agent.knowledge_files == null || agent.code_files == null;
let processed: ReturnType<typeof processAgentOption> | null = null;
if (needsHydration) {
const mergedFileMap = { ...fileMap };
for (const file of agentFiles) {
if (file.file_id) {
mergedFileMap[file.file_id] = file;
}
}
processed = processAgentOption({ agent, fileMap: mergedFileMap });
}
return {
contextFiles: agent.context_files ?? processed?.context_files ?? NO_FILES,
knowledgeFiles: agent.knowledge_files ?? processed?.knowledge_files ?? NO_FILES,
codeFiles: agent.code_files ?? processed?.code_files ?? NO_FILES,
};
}, [agent, agentId, fileMap, agentFiles]);
}
interface UseAgentItemsOptions {
agentId: string;
/** Skills to include in the catalog; omit to exclude the skill kind entirely. */
@ -120,7 +170,7 @@ export function useAgentItems({
const fileSearch = (useWatch({ control, name: 'file_search' }) ?? false) as boolean;
const memory = (useWatch({ control, name: 'memory' }) ?? false) as boolean;
const artifacts = (useWatch({ control, name: 'artifacts' }) ?? '') as string;
const agent = useWatch({ control, name: 'agent' });
const { contextFiles, knowledgeFiles, codeFiles } = useAgentFileEntries();
const agentActions = useMemo(
() => (actions ?? []).filter((a) => a.agent_id === agentId),
@ -154,8 +204,6 @@ export function useAgentItems({
],
);
/** Depends on the watched `agent` object rather than per-render derived file
* arrays, so the memo only recomputes when form state actually changes. */
const selected = useMemo(
() =>
deriveSelectedItems(
@ -167,9 +215,9 @@ export function useAgentItems({
artifacts,
tools,
skills: skillsField,
context_files: (agent?.context_files ?? []) as Array<[string, unknown]>,
knowledge_files: (agent?.knowledge_files ?? []) as Array<[string, unknown]>,
code_files: (agent?.code_files ?? []) as Array<[string, unknown]>,
context_files: contextFiles,
knowledge_files: knowledgeFiles,
code_files: codeFiles,
},
catalog,
agentActions,
@ -182,7 +230,9 @@ export function useAgentItems({
artifacts,
tools,
skillsField,
agent,
contextFiles,
knowledgeFiles,
codeFiles,
catalog,
agentActions,
],

View file

@ -1,7 +1,7 @@
import { AgentCapabilities, ArtifactModes } from 'librechat-data-provider';
import { computeToggleAction } from '../mutations';
import { makePlugin, makeSkill, makeMcpServer, makeAction } from 'test/itemFactories';
import type { AgentItem } from '../types';
import { makePlugin, makeSkill, makeMcpServer, makeAction } from 'test/itemFactories';
import { computeToggleAction, skillsEnabledTransition } from '../mutations';
const builtinCode: AgentItem = {
kind: 'builtin',
@ -130,3 +130,30 @@ describe('computeToggleAction', () => {
});
});
});
describe('skillsEnabledTransition', () => {
test('adding the first skill turns the master flag on', () => {
expect(skillsEnabledTransition([], ['s1'], undefined)).toBe(true);
expect(skillsEnabledTransition([], ['s1'], false)).toBe(true);
});
test('adding the first skill leaves an already-on flag alone', () => {
expect(skillsEnabledTransition([], ['s1'], true)).toBeUndefined();
});
test('removing the last skill turns the master flag off', () => {
expect(skillsEnabledTransition(['s1'], [], true)).toBe(false);
});
test('removing the last skill leaves an off flag alone', () => {
expect(skillsEnabledTransition(['s1'], [], false)).toBeUndefined();
expect(skillsEnabledTransition(['s1'], [], undefined)).toBeUndefined();
});
test('edits within a non-empty selection never touch the flag', () => {
expect(skillsEnabledTransition(['s1'], ['s1', 's2'], true)).toBeUndefined();
expect(skillsEnabledTransition(['s1'], ['s1', 's2'], false)).toBeUndefined();
expect(skillsEnabledTransition(['s1', 's2'], ['s1'], true)).toBeUndefined();
expect(skillsEnabledTransition(['s1', 's2'], ['s1'], false)).toBeUndefined();
});
});

View file

@ -45,3 +45,27 @@ export function computeToggleAction(item: AgentItem, state: { selected: boolean
? { type: 'action-remove', actionId: item.id }
: { type: 'action-add', actionId: item.id };
}
/**
* `skills_enabled` is the master opt-in for the skill allowlist, and an empty
* allowlist with the flag on means the FULL accessible catalog. Selection
* edits therefore sync the flag on empty/non-empty transitions: picking the
* first skill turns it on so the choice takes effect, and removing the last
* one turns it off so the agent doesn't silently escalate to every skill.
* Edits within a non-empty selection return `undefined` (leave the flag
* alone), preserving the Advanced kill switch's disable-without-clearing
* behavior.
*/
export function skillsEnabledTransition(
current: string[],
next: string[],
enabled: boolean | undefined,
): boolean | undefined {
if (current.length === 0 && next.length > 0 && enabled !== true) {
return true;
}
if (current.length > 0 && next.length === 0 && enabled === true) {
return false;
}
return undefined;
}