fix: strip legacy MCP tokens on removal, guard action creation, model button spacing

MCP selection accepts every historical token format (server placeholder,
raw server name, mcp_-prefixed, and per-tool ids in prefix/suffix shapes)
but removal only filtered the new placeholder plus the server's current
tool ids, so a legacy token left the server permanently selected and its
tools still expanded after save. Selection and removal now share a
matchesMcpServer predicate.

Creating an action from the marketplace on an unsaved agent opened an
editor whose save was guaranteed to fail; it now surfaces the existing
save-the-agent-first error, matching the action-removal guard.

The model picker button keeps its tight px-1 with a provider icon but gets
px-3 in the empty Select-a-model state so the placeholder is not flush
against the border.
This commit is contained in:
Marco Beretta 2026-07-03 22:45:32 +02:00
parent cf00c4fd83
commit 3a7cf02715
No known key found for this signature in database
GPG key ID: D918033D8E74CC11
4 changed files with 58 additions and 15 deletions

View file

@ -116,7 +116,10 @@ export default function AgentConfig() {
type="button"
onClick={() => setActivePanel(Panel.model)}
title={model || undefined}
className="relative flex h-9 w-full min-w-0 items-center overflow-hidden rounded-lg border border-border-light bg-surface-secondary px-1 text-sm font-medium text-text-primary transition-colors hover:bg-surface-tertiary focus:outline-none focus-visible:ring-2 focus-visible:ring-ring-primary"
className={cn(
'relative flex h-9 w-full min-w-0 items-center overflow-hidden rounded-lg border border-border-light bg-surface-secondary text-sm font-medium text-text-primary transition-colors hover:bg-surface-tertiary focus:outline-none focus-visible:ring-2 focus-visible:ring-ring-primary',
model != null && model ? 'px-1' : 'px-3',
)}
>
<div className="flex w-full min-w-0 items-center gap-2">
{Icon && (

View file

@ -7,12 +7,13 @@ import {
OGDialogTitle,
OGDialogContent,
OGDialogDescription,
useToastContext,
} from '@librechat/client';
import type { AgentItem, AgentItemKind, ItemFilter } from './items/types';
import type { AgentForm } from '~/common';
import { itemKey, mcpServerToken, matchesMcpServer } from './items/selectors';
import { useAgentItems, useUninstallToolCredentials } from './hooks';
import AddMcpServerDialog from './ItemDialog/AddMcpServerDialog';
import { itemKey, mcpServerToken } from './items/selectors';
import { computeToggleAction } from './items/mutations';
import { useLocalize, useToolFavorites } from '~/hooks';
import MarketplaceSidebar from './MarketplaceSidebar';
@ -20,6 +21,7 @@ import MarketplaceCatalog from './MarketplaceCatalog';
import ItemDialog from './ItemDialog/ItemDialog';
import { applyFilter } from './items/filtering';
import { NEW_ACTION_ID } from './items/types';
import { isEphemeralAgent } from '~/common';
interface ToolsMarketplaceDialogProps {
open: boolean;
@ -36,6 +38,7 @@ export default function ToolsMarketplaceDialog({
agentId,
}: ToolsMarketplaceDialogProps) {
const localize = useLocalize();
const { showToast } = useToastContext();
const { getValues, setValue } = useFormContext<AgentForm>();
const uninstallToolCredentials = useUninstallToolCredentials();
@ -56,6 +59,13 @@ export default function ToolsMarketplaceDialog({
setAddMcpOpen(true);
return;
}
/** Actions are persisted against the agent id, so an unsaved agent has
* nothing to attach them to surface the save-first error instead of
* letting the editor fail on submit. */
if (isEphemeralAgent(agentId)) {
showToast({ message: localize('com_agents_no_agent_id_error'), status: 'error' });
return;
}
setDetailItem({
kind: 'action',
id: NEW_ACTION_ID,
@ -65,7 +75,7 @@ export default function ToolsMarketplaceDialog({
endpointCount: 0,
});
},
[localize],
[agentId, localize, showToast],
);
const selectedIds = useMemo(() => new Set(selectedItems.map(itemKey)), [selectedItems]);
@ -123,11 +133,10 @@ export default function ToolsMarketplaceDialog({
case 'mcp-remove': {
if (item.kind !== 'mcp') break;
const toolIds = new Set((item.server.tools ?? []).map((t) => t.tool_id));
const serverToken = mcpServerToken(item.id);
const current = (getValues('tools') ?? []) as string[];
setValue(
'tools',
current.filter((t) => t !== serverToken && !toolIds.has(t)),
current.filter((t) => !matchesMcpServer(t, item.id) && !toolIds.has(t)),
{ shouldDirty: true },
);
break;

View file

@ -1,7 +1,7 @@
import { AgentCapabilities, Constants } from 'librechat-data-provider';
import type { AgentItem } from '../types';
import { makePlugin, makeSkill, makeMcpServer, makeAction } from 'test/itemFactories';
import { deriveSelectedItems } from '../selectors';
import { deriveSelectedItems, matchesMcpServer } from '../selectors';
const mcpServerToken = (serverName: string) =>
`${Constants.mcp_server}${Constants.mcp_delimiter}${serverName}`;
@ -197,3 +197,23 @@ describe('deriveSelectedItems', () => {
expect(kindOrder.indexOf('tool')).toBeLessThan(kindOrder.indexOf('skill'));
});
});
describe('matchesMcpServer', () => {
test('matches every persisted token format for the server', () => {
expect(matchesMcpServer(`${Constants.mcp_server}${Constants.mcp_delimiter}srv`, 'srv')).toBe(
true,
);
expect(matchesMcpServer('srv', 'srv')).toBe(true);
expect(matchesMcpServer('mcp_srv', 'srv')).toBe(true);
expect(matchesMcpServer('mcp_srv_tool', 'srv')).toBe(true);
expect(matchesMcpServer('search_mcp_srv', 'srv')).toBe(true);
expect(matchesMcpServer(`sys__all__sys${Constants.mcp_delimiter}srv`, 'srv')).toBe(true);
});
test('does not match tokens for other servers or plain tool ids', () => {
expect(matchesMcpServer('mcp_other', 'srv')).toBe(false);
expect(matchesMcpServer('search_mcp_other', 'srv')).toBe(false);
expect(matchesMcpServer('dalle', 'srv')).toBe(false);
expect(matchesMcpServer('srv2', 'srv')).toBe(false);
});
});

View file

@ -62,17 +62,28 @@ export function mcpServerToken(serverName: string): string {
return `${Constants.mcp_server}${Constants.mcp_delimiter}${serverName}`;
}
/**
* Whether a form `tools` token references the given MCP server, across every
* format ever persisted: the server placeholder token, the raw server name,
* the legacy `mcp_<server>` token, and per-tool ids in both prefix and suffix
* shapes. Selection and removal must share this predicate anything the
* selection logic counts as attached, an explicit remove must also strip, or
* a legacy token leaves the server permanently selected.
*/
export function matchesMcpServer(token: string, serverName: string): boolean {
const prefixed = `${MCP_PREFIX}${serverName}`;
return (
token === mcpServerToken(serverName) ||
token === serverName ||
token === prefixed ||
token.startsWith(`${prefixed}_`) ||
token.endsWith(`_${prefixed}`)
);
}
function isMcpSelected(item: AgentItem, form: FormSelection): boolean {
if (item.kind !== 'mcp') return false;
const prefixed = `${MCP_PREFIX}${item.id}`;
return form.tools.some(
(t) =>
t === mcpServerToken(item.id) ||
t === item.id ||
t === prefixed ||
t.startsWith(`${prefixed}_`) ||
t.endsWith(`_${prefixed}`),
);
return form.tools.some((t) => matchesMcpServer(t, item.id));
}
function isSkillSelected(item: AgentItem, form: FormSelection): boolean {