🧰 fix: Keep File-Backed Agent Capabilities and Their Saved Tools in Agreement (#15418)

An agent with knowledge files showed File Search as selected in the builder while
the save omitted `file_search` from its tools. At runtime the tool was gone, the row
still looked selected so re-adding it changed nothing, and the next save re-confirmed
the broken state. Recovery needed a MongoDB write.

The builder's model for file-backed capabilities is stated in ToolsSection: they stay
selected while they hold files, and removal is routed to the file manager rather than
the capability flag — mirroring the file-only `context` built-in. Three places
implemented that model separately and drifted:

- display resolved it from the flag OR the file count,
- persistence resolved it from the flag alone,
- the removal guard lived inline in the tools list and was missing entirely from the
  marketplace dialog.

That last gap is how the bad state was reached. `ToolsMarketplaceDialog` toggled a
selected File Search straight to `file_search: false` with no file check, producing
flag-off-with-files — a state display cannot show and persistence silently drops.

`items/capabilities.ts` now owns the capability table and the one rule, and display,
removal and persistence all derive from it:

- persistence resolves the same way display does, so a file-holding capability keeps
  its tool (this also heals already-broken agents on their next save),
- both toggle paths route removal of a file-holding built-in to the file dialog,
- the removal guard takes file counts rather than the agent, so a component can pass
  the hydrated entries it already watches while submit reads the form — the rule is
  shared, the source of the count need not be.

Submission also stops mutating `data.tools` in place and dedupes the result.

Supersedes the persistence half of #15380, which fixed the save but left the
marketplace path able to recreate the state and kept the rule in a third copy.

Fixes #15368
This commit is contained in:
Danny Avila 2026-08-31 09:47:32 -04:00 • committed by GitHub
parent 7b0b145159
commit ccd673bf05
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 343 additions and 42 deletions

View file

@ -5,7 +5,6 @@ import { Button, useToastContext } from '@librechat/client';
import { useWatch, useForm, FormProvider } from 'react-hook-form';
import { useGetModelsQuery } from 'librechat-data-provider/react-query';
import {
Tools,
MemoryScope,
SystemRoles,
ResourceType,
@ -36,6 +35,7 @@ import {
import { useResourcePermissions } from '~/hooks/useResourcePermissions';
import { useSelectAgent, useLocalize, useAuthContext } from '~/hooks';
import { useAgentPanelContext } from '~/Providers/AgentPanelContext';
import { resolveCapabilityTools } from './Tools/items/capabilities';
import AgentPanelSkeleton from './AgentPanelSkeleton';
import AdvancedPanel from './Advanced/AdvancedPanel';
import { Panel, isEphemeralAgent } from '~/common';
@ -564,20 +564,7 @@ export default function AgentPanel() {
const onSubmit = useCallback(
async (data: AgentForm) => {
const tools = data.tools ?? [];
if (data.execute_code === true) {
tools.push(Tools.execute_code);
}
if (data.file_search === true) {
tools.push(Tools.file_search);
}
if (data.web_search === true) {
tools.push(Tools.web_search);
}
if (data.memory === true) {
tools.push(Tools.memory);
}
const tools = Array.from(new Set([...(data.tools ?? []), ...resolveCapabilityTools(data)]));
const { payload: basePayload, provider, model } = composeAgentUpdatePayload(data, agent_id);

View file

@ -11,6 +11,7 @@ import {
useToastContext,
} from '@librechat/client';
import type { AgentItem, AgentItemKind, ItemFilter } from './items/types';
import type { CapabilityFileCounts } from './items/capabilities';
import type { AgentForm } from '~/common';
import {
itemKey,
@ -19,7 +20,8 @@ import {
matchesMcpServer,
mcpServerIds,
} from './items/selectors';
import { useAgentItems, useUninstallToolCredentials } from './hooks';
import { useAgentFileEntries, useAgentItems, useUninstallToolCredentials } from './hooks';
import { requiresFileManagerRemoval } from './items/capabilities';
import AddMcpServerDialog from './ItemDialog/AddMcpServerDialog';
import { computeToggleAction } from './items/mutations';
import { useLocalize, useToolFavorites } from '~/hooks';
@ -86,6 +88,11 @@ export default function ToolsMarketplaceDialog({
);
const selectedIds = useMemo(() => new Set(selectedItems.map(itemKey)), [selectedItems]);
const { knowledgeFiles, codeFiles } = useAgentFileEntries();
const fileCounts: CapabilityFileCounts = useMemo(
() => ({ knowledge_files: knowledgeFiles.length, code_files: codeFiles.length }),
[knowledgeFiles, codeFiles],
);
const counts = useMemo(
() => ({
@ -106,7 +113,15 @@ export default function ToolsMarketplaceDialog({
const handleToggle = useCallback(
(item: AgentItem) => {
const patch = computeToggleAction(item, { selected: selectedIds.has(itemKey(item)) });
const selected = selectedIds.has(itemKey(item));
/** A file-backed built-in cannot be switched off by its flag while it holds
* files — the row would stay selected and the save would disagree with it — so
* send the user to the dialog that manages those files, as the tools list does. */
if (selected && item.kind === 'builtin' && requiresFileManagerRemoval(item.id, fileCounts)) {
setDetailItem(item);
return;
}
const patch = computeToggleAction(item, { selected });
switch (patch.type) {
case 'builtin':
setValue(patch.field as keyof AgentForm, patch.value as never, { shouldDirty: true });
@ -160,7 +175,7 @@ export default function ToolsMarketplaceDialog({
break;
}
},
[getValues, setValue, selectedIds, uninstallToolCredentials, catalog],
[getValues, setValue, selectedIds, uninstallToolCredentials, catalog, fileCounts],
);
const handleCardClick = useCallback(

View file

@ -19,16 +19,18 @@ import {
} from '@librechat/client';
import type { TPlugin } from 'librechat-data-provider';
import type { ReactNode } from 'react';
import type { CapabilityFileCounts } from './items/capabilities';
import type { AgentItem } from './items/types';
import type { AgentForm } from '~/common';
import {
useAgentFileEntries,
useAgentItems,
useResolvedSkills,
useAgentFileEntries,
useUninstallToolCredentials,
} from './hooks';
import { computeToggleAction, skillsEnabledTransition } from './items/mutations';
import { useListSkillsQuery, useDeleteAgentAction } from '~/data-provider';
import { requiresFileManagerRemoval } from './items/capabilities';
import { useRemoveMCPTool, useVisibleTools } from '~/hooks/MCP';
import ToolsMarketplaceDialog from './ToolsMarketplaceDialog';
import { useLocalize, useHasAccess } from '~/hooks';
@ -117,7 +119,6 @@ export default function ToolsSection({ agentId }: Props) {
);
const uninstallToolCredentials = useUninstallToolCredentials();
const { knowledgeFiles, codeFiles } = useAgentFileEntries();
const { selected, tools } = useAgentItems({
agentId,
@ -125,27 +126,16 @@ export default function ToolsSection({ agentId }: Props) {
skillsPermission: showSkills,
});
/** File-backed built-ins stay selected while they hold files, so flipping
* the capability flag off would leave the row visible. Route their removal
* to the config dialog (where files are managed) instead, mirroring the
* file-only `context` built-in. */
const { knowledgeFiles, codeFiles } = useAgentFileEntries();
const fileCounts: CapabilityFileCounts = useMemo(
() => ({ knowledge_files: knowledgeFiles.length, code_files: codeFiles.length }),
[knowledgeFiles, codeFiles],
);
const opensFileManagerOnRemove = useCallback(
(item: AgentItem): boolean => {
if (item.kind !== 'builtin') {
return false;
}
if (item.id === 'context') {
return true;
}
if (item.id === 'execute_code') {
return codeFiles.length > 0;
}
if (item.id === 'file_search') {
return knowledgeFiles.length > 0;
}
return false;
},
[codeFiles, knowledgeFiles],
(item: AgentItem): boolean =>
item.kind === 'builtin' && requiresFileManagerRemoval(item.id, fileCounts),
[fileCounts],
);
const handleQuickRemove = useCallback(

View file

@ -69,11 +69,18 @@ jest.mock('react-router-dom', () => ({
useNavigate: () => jest.fn(),
}));
let mockFileEntries: {
contextFiles: Array<[string, unknown]>;
knowledgeFiles: Array<[string, unknown]>;
codeFiles: Array<[string, unknown]>;
} = { contextFiles: [], knowledgeFiles: [], codeFiles: [] };
jest.mock('../hooks', () => {
const { buildCatalog } = jest.requireActual('../items/catalog');
const { deriveSelectedItems } = jest.requireActual('../items/selectors');
return {
useUninstallToolCredentials: () => jest.fn(),
useAgentFileEntries: () => mockFileEntries,
/** Mirrors the real pipeline over the mocked panel context + form watches. */
useAgentItems: ({ agentId }: { agentId: string }) => {
const { useAgentPanelContext } = jest.requireMock('~/Providers');
@ -161,6 +168,7 @@ describe('ToolsMarketplaceDialog', () => {
mockMcpServersMap = new Map();
mockToggleFavorite.mockClear();
mockFavoriteKeys = new Set<string>();
mockFileEntries = { contextFiles: [], knowledgeFiles: [], codeFiles: [] };
});
test('renders cards from catalog when open', () => {
@ -185,6 +193,18 @@ describe('ToolsMarketplaceDialog', () => {
);
});
test('routes a file-holding built-in to its file dialog instead of clearing the flag', () => {
/** Clearing the flag here left the row still selected (it reads the file count) while
* the save wrote the tool off the flag alone, silently dropping it from the agent. */
mockExecuteCode = true;
mockFileEntries = { contextFiles: [], knowledgeFiles: [], codeFiles: [['c1', {}]] };
render(<ToolsMarketplaceDialog open onOpenChange={jest.fn()} agentId="a1" />);
fireEvent.click(screen.getByRole('button', { name: /com_ui_run_code/ }));
expect(mockSetValue).not.toHaveBeenCalledWith('execute_code', false, expect.anything());
});
test('disabling Code Interpreter clears programmatic MCP callers immediately', () => {
mockExecuteCode = true;
mockGetValues.mockImplementation((name: string) =>

View file

@ -0,0 +1,177 @@
import { AgentCapabilities, EToolResources, Tools } from 'librechat-data-provider';
import type { AgentForm, TAgentOption } from '~/common';
import {
countCapabilityFiles,
FILE_BACKED_CAPABILITIES,
isFileBackedCapabilityEnabled,
requiresFileManagerRemoval,
resolveCapabilityTools,
} from '../capabilities';
const fileEntry = (id: string) => [id, { file_id: id }] as [string, never];
const agentWith = (overrides: Partial<TAgentOption> = {}): TAgentOption =>
({ id: 'agent_1', ...overrides }) as TAgentOption;
const formWith = (overrides: Partial<AgentForm> = {}): AgentForm =>
({ id: 'agent_1', tools: [], ...overrides }) as AgentForm;
const knowledge = FILE_BACKED_CAPABILITIES.find(
(entry) => entry.capability === AgentCapabilities.file_search,
)!;
describe('countCapabilityFiles', () => {
it('counts the hydrated form entries', () => {
const agent = agentWith({ knowledge_files: [fileEntry('f1'), fileEntry('f2')] });
expect(countCapabilityFiles(agent, knowledge)).toBe(2);
});
it('falls back to persisted ids before the file map hydrates', () => {
const agent = agentWith({
tool_resources: { [EToolResources.file_search]: { file_ids: ['f1', 'f2', 'f3'] } },
});
expect(countCapabilityFiles(agent, knowledge)).toBe(3);
});
it('prefers hydrated entries over persisted ids', () => {
const agent = agentWith({
knowledge_files: [fileEntry('f1')],
tool_resources: { [EToolResources.file_search]: { file_ids: ['f1', 'f2', 'f3'] } },
});
expect(countCapabilityFiles(agent, knowledge)).toBe(1);
});
it('reports no files for an agent that holds none', () => {
expect(countCapabilityFiles(agentWith(), knowledge)).toBe(0);
expect(countCapabilityFiles(undefined, knowledge)).toBe(0);
});
});
describe('isFileBackedCapabilityEnabled', () => {
it('is on when the flag is set', () => {
expect(isFileBackedCapabilityEnabled(true, 0)).toBe(true);
});
it('is on when files are attached without the flag', () => {
expect(isFileBackedCapabilityEnabled(false, 1)).toBe(true);
expect(isFileBackedCapabilityEnabled(undefined, 1)).toBe(true);
});
it('is off only with neither', () => {
expect(isFileBackedCapabilityEnabled(false, 0)).toBe(false);
expect(isFileBackedCapabilityEnabled(undefined, 0)).toBe(false);
});
});
describe('resolveCapabilityTools', () => {
it('persists file_search for an agent holding knowledge files with the flag cleared', () => {
const data = formWith({
[AgentCapabilities.file_search]: false,
agent: agentWith({ knowledge_files: [fileEntry('f1')] }),
});
expect(resolveCapabilityTools(data)).toContain(Tools.file_search);
});
it('persists file_search from the flag alone before any file is attached', () => {
const data = formWith({ [AgentCapabilities.file_search]: true, agent: agentWith() });
expect(resolveCapabilityTools(data)).toEqual([Tools.file_search]);
});
it('persists execute_code for an agent holding code files with the flag cleared', () => {
const data = formWith({
[AgentCapabilities.execute_code]: false,
agent: agentWith({ code_files: [fileEntry('c1')] }),
});
expect(resolveCapabilityTools(data)).toContain(Tools.execute_code);
});
it('resolves file-backed capabilities from the unhydrated agent document', () => {
const data = formWith({
[AgentCapabilities.file_search]: false,
agent: agentWith({
tool_resources: { [EToolResources.file_search]: { file_ids: ['f1'] } },
}),
});
expect(resolveCapabilityTools(data)).toContain(Tools.file_search);
});
it('omits capabilities that are neither enabled nor backed by files', () => {
expect(resolveCapabilityTools(formWith({ agent: agentWith() }))).toEqual([]);
});
it('persists flag-only capabilities from their flag alone', () => {
const data = formWith({
[AgentCapabilities.web_search]: true,
[AgentCapabilities.memory]: true,
agent: agentWith(),
});
expect(resolveCapabilityTools(data)).toEqual([Tools.web_search, Tools.memory]);
});
it('does not resurrect a flag-only capability from attached files', () => {
const data = formWith({
[AgentCapabilities.web_search]: false,
agent: agentWith({ knowledge_files: [fileEntry('f1')] }),
});
expect(resolveCapabilityTools(data)).not.toContain(Tools.web_search);
});
});
describe('requiresFileManagerRemoval', () => {
const counts = (knowledge = 0, code = 0) => ({
knowledge_files: knowledge,
code_files: code,
});
it('routes a file-holding capability to the file manager', () => {
expect(requiresFileManagerRemoval(AgentCapabilities.file_search, counts(1))).toBe(true);
expect(requiresFileManagerRemoval(AgentCapabilities.execute_code, counts(0, 1))).toBe(true);
});
it('lets a capability with no files be switched off by its flag', () => {
expect(requiresFileManagerRemoval(AgentCapabilities.file_search, counts())).toBe(false);
expect(requiresFileManagerRemoval(AgentCapabilities.execute_code, counts(1))).toBe(false);
});
it('always routes the file-only context built-in to the file manager', () => {
expect(requiresFileManagerRemoval(EToolResources.context, counts())).toBe(true);
});
it('leaves flag-only built-ins alone', () => {
expect(requiresFileManagerRemoval(AgentCapabilities.web_search, counts(1, 1))).toBe(false);
expect(requiresFileManagerRemoval('artifacts', counts(1, 1))).toBe(false);
});
});
describe('display and persistence agree', () => {
/** The reported bug: the builder showed File Search as selected off the file count
* while the save wrote it off the flag alone, so it was dropped from an agent that
* still listed its knowledge files. Both sides now read the same rule. */
it.each([
[true, 0],
[false, 1],
[true, 1],
[false, 0],
])('flag=%s files=%s', (flag, fileCount) => {
const displayed = isFileBackedCapabilityEnabled(flag, fileCount);
const persisted = resolveCapabilityTools(
formWith({
[AgentCapabilities.file_search]: flag,
agent: agentWith({
knowledge_files: Array.from({ length: fileCount }, (_, i) => fileEntry(`f${i}`)),
}),
}),
).includes(Tools.file_search);
expect(persisted).toBe(displayed);
});
});

View file

@ -0,0 +1,109 @@
import { AgentCapabilities, EToolResources, Tools } from 'librechat-data-provider';
import type { AgentForm, TAgentOption } from '~/common';
/**
* Built-in capabilities whose selection is backed by attached files. These are on
* while they hold files: the builder routes their removal to the file manager rather
* than the capability flag, so "flag off while files remain" is a state the UI cannot
* show and the user cannot reach deliberately.
*
* Display, removal and persistence all derive from this table. They used to encode the
* rule separately and drifted — the builder showed File Search as selected off the file
* count while the save wrote it off the flag alone, so the tool was silently dropped
* from an agent that still listed its knowledge files.
*/
export const FILE_BACKED_CAPABILITIES = [
{
capability: AgentCapabilities.file_search,
tool: Tools.file_search,
files: 'knowledge_files',
resource: EToolResources.file_search,
},
{
capability: AgentCapabilities.execute_code,
tool: Tools.execute_code,
files: 'code_files',
resource: EToolResources.execute_code,
},
] as const;
export type FileBackedCapability = (typeof FILE_BACKED_CAPABILITIES)[number];
/** Built-in capabilities that are a plain on/off flag, with no files behind them. */
const FLAG_ONLY_CAPABILITIES = [
{ capability: AgentCapabilities.web_search, tool: Tools.web_search },
{ capability: AgentCapabilities.memory, tool: Tools.memory },
] as const;
/**
* How many files an agent holds for a file-backed capability. Prefers the hydrated
* form entries and falls back to the persisted ids, which are all that exist before
* the file map hydrates — a save during that window must not read "no files" and
* drop the tool.
*/
export function countCapabilityFiles(
agent: TAgentOption | undefined,
{ files, resource }: Pick<FileBackedCapability, 'files' | 'resource'>,
): number {
const entries = agent?.[files];
if (entries != null) {
return entries.length;
}
return agent?.tool_resources?.[resource]?.file_ids?.length ?? 0;
}
/** Whether a file-backed capability is on, given its flag and the files it holds. */
export function isFileBackedCapabilityEnabled(flag: boolean | undefined, fileCount: number) {
return flag === true || fileCount > 0;
}
/** File counts per capability, supplied by the caller from its best source. */
export type CapabilityFileCounts = Record<FileBackedCapability['files'], number>;
/**
* Whether removing this built-in has to go through the file manager. A capability
* holding files cannot be switched off by clearing its flag — the row would stay
* selected and the save would disagree with it — so the caller opens the dialog
* where the files themselves are managed.
*
* Takes counts rather than the agent so a component can pass the hydrated entries it
* already watches, while `resolveCapabilityTools` reads the form on submit where no
* hook is available. The rule is shared; the source of the count need not be.
*/
export function requiresFileManagerRemoval(itemId: string, counts: CapabilityFileCounts): boolean {
if (itemId === EToolResources.context) {
return true;
}
const entry = FILE_BACKED_CAPABILITIES.find(({ capability }) => capability === itemId);
if (entry == null) {
return false;
}
return counts[entry.files] > 0;
}
/**
* The built-in capability tools a submission should persist, resolved with the same
* rule the builder displays. Returned separately from `data.tools` so the caller
* decides how to merge them.
*/
export function resolveCapabilityTools(data: AgentForm): string[] {
const tools: string[] = [];
for (const entry of FILE_BACKED_CAPABILITIES) {
const enabled = isFileBackedCapabilityEnabled(
data[entry.capability],
countCapabilityFiles(data.agent, entry),
);
if (enabled) {
tools.push(entry.tool);
}
}
for (const { capability, tool } of FLAG_ONLY_CAPABILITIES) {
if (data[capability] === true) {
tools.push(tool);
}
}
return tools;
}

View file

@ -6,6 +6,7 @@ import {
} from 'librechat-data-provider';
import type { Action } from 'librechat-data-provider';
import type { AgentItem, AgentItemKind } from './types';
import { isFileBackedCapabilityEnabled } from './capabilities';
export interface FormSelection {
execute_code: boolean;
@ -35,12 +36,14 @@ export function itemKey(item: Pick<AgentItem, 'kind' | 'id'>): string {
function isBuiltinSelected(item: AgentItem, form: FormSelection): boolean {
if (item.kind !== 'builtin') return false;
switch (item.id) {
/** File-backed built-ins share their on/off rule with removal and persistence
* (see `capabilities.ts`), so the three cannot drift apart again. */
case 'execute_code':
return form.execute_code || form.code_files.length > 0;
return isFileBackedCapabilityEnabled(form.execute_code, form.code_files.length);
case 'web_search':
return form.web_search;
case 'file_search':
return form.file_search || form.knowledge_files.length > 0;
return isFileBackedCapabilityEnabled(form.file_search, form.knowledge_files.length);
case 'memory':
return form.memory;
case 'artifacts':