mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 06:52:47 +00:00
🔗 fix: Resolve MCP Tool-Key Boundary Against Configured Server Names (#14448)
* fix: resolve MCP tool-name delimiter collision at invocation time
MCP tool keys are identified internally as `${rawToolName}${mcp_delimiter}${serverName}`
(delimiter `_mcp_`). Several call sites parsed this back apart with a naive
`toolKey.split(Constants.mcp_delimiter)`, assuming the delimiter occurs exactly once.
When the raw upstream tool name itself contains the delimiter substring - which
happens whenever it's exposed through a gateway that prefixes aggregated tool names by
server (e.g. a gateway's own "gitlab-get_mcp_server_version" for GitLab's
"get_mcp_server_version" tool) - the combined key has the delimiter more than once.
`.split()` then produces more than two segments, and destructuring
`[toolName, serverName]` silently keeps only the first two, yielding a bogus server
name that matches no configured server. Tool listing still worked (a different code
path builds keys directly without re-splitting), but invocation failed with
`Tool {name} not found`, and `filterAuthorizedTools` rejected such keys outright as
malformed.
Add `splitMCPToolKey`, which splits on the *last* occurrence of the delimiter instead:
the server-name half is always LibreChat's own normalized suffix (guaranteed not to
contain the delimiter), while the raw tool-name half is untrusted and may legitimately
contain it. This matches `.split()`'s result whenever the delimiter occurs once, and
correctly resolves the collision case. Update the four call sites that parsed this
manually (`handleTools.js`, `MCP.js`, `mcp.js` controller, `filterAuthorizedTools` in
`v1.js`) plus one in the client (`useVisibleTools.ts`) to use it.
Fixes #14440
* fix: resolve MCP tool-key boundary against configured server names
splitMCPToolKey moves to librechat-data-provider so the client and backend
share one parser, and takes the configured server names when the caller has
them: the longest name the key actually ends with wins, which is exact.
Position alone cannot identify the boundary because both halves may contain
the delimiter. lastIndexOf alone fixes gateway-prefixed tool names but
regresses servers whose own name contains it, which ToolService.spec.js
already covered; the last-delimiter path now only serves as the fallback for
callers with no configured set.
Also converts the remaining first-occurrence parsers that the delimiter fix
missed - mcp/auth.ts (custom user vars silently unresolved), mcp/oauth/events.ts,
agents/initialize.ts, and the three client parsers that labelled tool calls
with the wrong server.
* fix: keep client tool-call labels on first-delimiter parsing
The three client parsers had deliberate, tested first-delimiter semantics
(ToolCall.test.tsx asserts the full server name for 'foo_mcp_bar' and the
synthetic 'oauth_mcp_server' call), and the client has no configured server
list in scope to resolve the boundary exactly, so they are left as they were.
Threads the configured names into the event-driven definition loader so it
resolves the same boundary as the authorization filter that admits the key,
and documents the one case that stays undecidable without provenance.
* fix: resolve tool-key boundary against all configured servers
resolveConfigServers only returns lazily-initialized config overrides -
ensureConfigServers skips unmodified YAML servers - so on a stock deployment
the known-name list was empty and suffix resolution never engaged. Adds
resolveMcpServerNames, which keeps every configured server in the normalized
form tool keys carry, and uses it at the loading, auth-map and definition
sites.
Background-tool eligibility now resolves against all configured names before
testing ephemeral membership, so a non-ephemeral server whose name ends in an
ephemeral one is no longer misclassified, and useVisibleTools resolves against
the server map it already receives.
* fix: use resolved server provenance and one app-config read
createMCPTool now uses the serverName loadTools already resolved for the key
and only parses as a fallback, so an unmodified YAML server whose name
contains the delimiter no longer resolves to the wrong server for auth,
reconnection and callTool.
resolveMcpServerContext derives config servers and all configured names from
a single getAppConfigForRequest, replacing two independent lookups on the
chat startup path, and degrades to empty like resolveConfigServers instead of
aborting tool loading when the config lookup fails.
* chore: drop unused resolveConfigServers import
* fix: forward server provenance on the all-tools path and read config once
createMCPTools builds each toolKey from the server name it already has but did
not forward it, so the sys__all__sys path re-derived it by parsing and bound
an unmodified YAML server whose name contains the delimiter to the wrong auth
and invocation context.
loadAgentTools now resolves the MCP server context once and threads it into
loadTools, replacing the second app-config read it had introduced on the
non-event-driven chat startup path.
* fix: carry resolved MCP server name through tool classification
definitions.ts resolves the server for each key and then dropped it when
building loadedTools, so buildToolClassification re-derived it with a
last-segment split and recorded 'Workspace' for a server configured as
'Google_mcp_Workspace'. The resolved name now rides along on the tool
instance and classification prefers it over re-parsing.
* fix: consume carried server name when extracting MCP servers
extractMCPServers re-derived the name with a last-segment split, so a server
configured as Google_mcp_Workspace resolved to Workspace and its instructions
were silently omitted. Prefers the name carried on the tool definition
instance, falling back to the split.
* fix: fail closed on ambiguous MCP keys when persisting server names
Persisted mcpServerNames grant agent-scoped access to a DB server by name
(ServerConfigsDB.getAccessibleServers), so a wrong guess exposes an unrelated
server to everyone who can view the agent. The last-segment split turned
search_mcp_Google_mcp_workspace into 'workspace'; such keys were previously
rejected outright at agent save, so admitting them opened this path.
Derives a name only from unambiguous single-delimiter keys. This is #12250's
guard moved to the boundary it was actually protecting, instead of blocking
tool admission.
* fix: keep DB server access for multi-delimiter tool keys
The fail-closed guard was wrong for the case this PR exists to fix. This index
only grants DB-backed servers, and DB names are slugs that cannot contain the
delimiter (generateServerNameFromTitle strips underscores), so the trailing
segment is always the real server for them - dropping it cost every consumer
of a gateway-prefixed tool their shared-agent access.
Also gates the MCP server-context lookup on the filtered MCP set, so an agent
with no MCP tools no longer pays an app-config read on startup.
* fix: resolve tool-call display names without breaking OAuth calls
The display parsers could not use the shared boundary parser because their
tested behavior depends on first-delimiter semantics. That constraint only
applies to synthetic MCP OAuth calls, whose tool half is always exactly
'oauth', so everything after the first delimiter is the server even when the
server name carries one.
splitToolCallName special-cases that form and defers to splitMCPToolKey for
real tool keys, so a gateway-prefixed tool now renders its own name and
server while oauth_mcp_foo_mcp_bar still resolves to foo_mcp_bar.
* fix: persist resolved MCP server provenance on agents
Deriving mcpServerNames from the tool key cannot tell a config server's
trailing segment from a real DB server name, so a config server named
a_mcp_b indexed an unrelated DB server b and shared the agent's viewers into
it. Neither string rule works: the suffix guess exposes, and failing closed
drops legitimate DB access for gateway-prefixed tools.
filterAuthorizedTools already resolves each tool's server against the merged
registry config, so it now collects those names and create, update and
duplicate persist them. No extra registry queries: the update path unions the
newly resolved names with what the agent already had, and duplicate replaces
the copied list rather than inheriting the source's servers.
Display parsing also takes the configured names, so a real tool call on a
delimiter-bearing server renders the right server and icon.
* test: teach MCP hook mocks about useMCPServerNames
Three specs mock ~/hooks/MCP with a hand-listed factory, so adding the hook
to ToolCall made useMCPServerNames undefined under test and every render
threw. Returns a stable array so the mock cannot perturb render counts.
* fix: rebuild agent MCP server index from surviving tools
Unioning the prior names kept a server indexed after its last tool was
detached, so viewers of a shared agent retained agent-scoped access to it.
The index is now rebuilt from the tools that survive the edit: a prior name
carries forward only while some retained tool still resolves to it, using the
agent's own persisted names as the candidate set, and the rebuild runs on any
tool change rather than only when a new MCP tool is added.
* fix: keep duplicate indexes on registry fallback and harden the oauth split
Duplication blanked mcpServerNames when the registry was unavailable, because
filterAuthorizedTools grandfathers the source's tools without resolving them -
the copy kept tools it could no longer resolve. Source names now carry forward
for the tools that still point at them.
splitToolCallName also treated any oauth_mcp_ prefix as a synthetic OAuth
call, so a genuine upstream tool by that name resolved to the wrong server. A
configured server name now decides when one matches, since a real key always
ends in its server, and the prefix only breaks ties for unconfigured servers.
* fix: thread configured server names through display parsing
parseToolName and getMCPServerName resolved context-free, so a configured
server whose name contains the delimiter showed the wrong server in grouped
tool summaries and subagent tool labels, and stacked icons missed its entry in
the icon map. Both take the configured names now, supplied by the components
that render them.
Adds the hook to SubagentCall's mock factory: the spec renders the real
component, so an unmocked useMCPServerNames would reach the query with no
provider.
* test: cover the auth-map boundary, server provenance and context fallback
Adds regression coverage for three behaviors this PR changed that no test
exercised: customUserVars resolving under the right plugin key for a
gateway-prefixed tool name (the failure that made these tools loadable but
unusable), the resolved server name reaching createMCPTool instead of being
re-parsed, and resolveMcpServerContext degrading to empty rather than
aborting tool loading when the config lookup fails.
Each was checked against a mutated source to confirm it fails when the
behavior is broken.
* fix: normalize server-name candidates and cover the boundary guard
Tool keys embed normalizeServerName's output while the config is keyed by the
raw name, so callers passing raw keys never matched a server whose name needs
normalizing and silently fell back to the last delimiter. filterAuthorizedTools
now maps normalized names back to their config key, and createMCPTool
normalizes its candidates.
Adds the cases an audit found surviving mutation: a configured name that is a
bare but not delimiter-aligned suffix must not match, an empty candidate list
behaves as no list, and splitToolCallName still falls back to the oauth prefix
when a list is supplied but nothing in it matches.
* fix: keep resolved server names when a non-owner retains MCP tools
The shared-agent path keeps an agent's existing MCP tools verbatim but supplied
no mcpServerNames, so persistence re-derived them and reduced a configured
server like Google_mcp_Workspace to Workspace - which ServerConfigsDB then
treats as a DB server, granting the agent's viewers access to an unrelated one.
Carries the existing resolved names across instead, and clears the index on the
owner path where every MCP tool is removed.
* fix: preserve resolved MCP names for every tools update
extractMCPServerNames was reachable from any caller that writes tools without
mcpServerNames - the Action edit path does exactly that - so a configured
Google_mcp_Workspace was reindexed as Workspace and ServerConfigsDB granted
shared-agent viewers an unrelated DB server by that name.
updateAgent now rebuilds the index from the agent's own resolved names: one
carries forward while a retained tool still resolves to it, and only keys
matching none of them fall back to derivation. Callers are safe by default
rather than by remembering to pass the set.
normalizeServerName moves to librechat-data-provider so the client can match
its candidates against tool keys, which embed the normalized form; the icon map
is keyed the same way since it is looked up with a parsed server name.
* refactor: move MCP context resolution into packages/api
New backend logic belongs in the TypeScript workspace per CLAUDE.md, with /api
kept to a thin wrapper. resolveMCPServerContext now lives in
packages/api/src/mcp/context.ts and takes ensureConfigServers by injection,
since the registry accessor is still legacy-only; the /api function is reduced
to loading the request app config and translating failures into the empty
degrade it already promised.
* test: teach the MCP service mock about resolveMCPServerContext
The spec mocks @librechat/api with a hand-listed factory, so moving the
resolver into that package left it undefined and the wrapper degraded into its
own catch, returning empty config servers. The stub mirrors the real resolver
so these tests still cover what the wrapper owns - loading the request config
and degrading on failure - while the resolution logic is unit-tested in
packages/api.
* fix: only persist an authoritative MCP server index on update
Assigning the resolved set unconditionally pinned the index to [] whenever
nothing authoritative was available - a legacy agent holding MCP tools with no
stored mcpServerNames - which suppressed updateAgent's derivation and stripped
agent-scoped access to its DB-backed server.
The field is now supplied only when the result is authoritative: names were
resolved, or no MCP tool survives so the index genuinely is empty. The
retained-tools branch likewise leaves it unset when the agent has none stored.
---------
Co-authored-by: Jens Schumann <schumajs@gmail.com>
This commit is contained in:
Danny Avila
GitHub
parent
74f46f90a1
commit
250aca375a
39 changed files with 1052 additions and 125 deletions
|
|
@ -6,6 +6,7 @@ const {
|
|||
createSafeUser,
|
||||
mcpToolPattern,
|
||||
loadWebSearchAuth,
|
||||
splitMCPToolKey,
|
||||
buildInlineMemoryTool,
|
||||
getCodeApiAuthHeaders,
|
||||
buildImageToolContext,
|
||||
|
|
@ -45,7 +46,7 @@ const {
|
|||
createMCPTool,
|
||||
createMCPTools,
|
||||
createMCPPermissionContext,
|
||||
resolveConfigServers,
|
||||
resolveMcpServerContext,
|
||||
} = require('~/server/services/MCP');
|
||||
const { getMCPRequestContext } = require('~/server/services/MCPRequestContext');
|
||||
const { createFileSearchTool, primeFiles: primeSearchFiles } = require('./fileSearch');
|
||||
|
|
@ -285,8 +286,13 @@ const loadTools = async ({
|
|||
|
||||
/** Resolve config-source servers for the current user/tenant context */
|
||||
let configServers;
|
||||
/** All configured names, in the normalized form tool keys carry */
|
||||
let mcpServerNames = [];
|
||||
if (hasMCPTools && canUseMCP) {
|
||||
configServers = await resolveConfigServers(options.req);
|
||||
/** Reuse the caller's context when it already resolved one, so the chat
|
||||
* startup path reads the request app config once. */
|
||||
({ configServers, serverNames: mcpServerNames } =
|
||||
options.mcpServerContext ?? (await resolveMcpServerContext(options.req)));
|
||||
}
|
||||
|
||||
for (const tool of tools) {
|
||||
|
|
@ -396,7 +402,7 @@ const loadTools = async ({
|
|||
continue;
|
||||
}
|
||||
|
||||
const [toolName, serverName] = tool.split(Constants.mcp_delimiter);
|
||||
const [toolName, serverName] = splitMCPToolKey(tool, mcpServerNames);
|
||||
if (toolName === Constants.mcp_server) {
|
||||
/** Placeholder used for UI purposes */
|
||||
continue;
|
||||
|
|
|
|||
|
|
@ -42,6 +42,7 @@ jest.mock('~/server/services/MCP', () => ({
|
|||
canUseServers: jest.fn().mockResolvedValue(true),
|
||||
})),
|
||||
resolveConfigServers: jest.fn().mockResolvedValue({}),
|
||||
resolveMcpServerContext: jest.fn(async () => ({ configServers: {}, serverNames: [] })),
|
||||
}));
|
||||
|
||||
jest.mock('~/config', () => ({
|
||||
|
|
@ -357,6 +358,54 @@ describe('Tool Handlers', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('resolves an MCP tool whose raw name itself contains the delimiter substring', async () => {
|
||||
// Regression test for https://github.com/danny-avila/LibreChat/issues/14440:
|
||||
// gateways that prefix aggregated tool names by server (e.g. LiteLLM's
|
||||
// MCP proxy) can produce a raw tool name that already contains "_mcp_"
|
||||
// (e.g. GitLab's own "get_mcp_server_version" tool becomes
|
||||
// "gitlab-get_mcp_server_version" once gateway-prefixed). Once
|
||||
// LibreChat appends its own server suffix, the combined key has the
|
||||
// delimiter twice - a naive split used to silently derive the wrong
|
||||
// server name ("server_version" instead of "gitlab") and drop the tool.
|
||||
const serverName = 'gitlab';
|
||||
const rawToolName = 'gitlab-get_mcp_server_version';
|
||||
const toolKey = `${rawToolName}${Constants.mcp_delimiter}${serverName}`;
|
||||
const serverConfig = {
|
||||
type: 'streamable-http',
|
||||
url: 'https://litellm.example.com/gitlab/mcp',
|
||||
source: 'yaml',
|
||||
};
|
||||
|
||||
mockGetServerConfig.mockResolvedValue(serverConfig);
|
||||
mockCreateMCPTool.mockResolvedValue({ name: 'loaded-mcp-tool' });
|
||||
|
||||
const result = await loadTools({
|
||||
user: fakeUser._id.toString(),
|
||||
tools: [toolKey],
|
||||
options: {
|
||||
req: {
|
||||
user: { id: fakeUser._id.toString(), role: 'USER' },
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
expect(result.loadedTools).toEqual([{ name: 'loaded-mcp-tool' }]);
|
||||
expect(mockGetServerConfig).toHaveBeenCalledWith(
|
||||
serverName,
|
||||
expect.anything(),
|
||||
expect.anything(),
|
||||
);
|
||||
expect(mockCreateMCPTool).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
toolKey,
|
||||
config: serverConfig,
|
||||
/** The resolved server rides along, so `createMCPTool` uses it for auth,
|
||||
* reconnection and invocation instead of re-parsing the ambiguous key. */
|
||||
serverName,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('uses run-scoped MCP tool definitions before cache lookup', async () => {
|
||||
const serverName = 'body-scoped';
|
||||
const toolKey = `search${Constants.mcp_delimiter}${serverName}`;
|
||||
|
|
|
|||
|
|
@ -342,23 +342,58 @@ describe('MCP Tool Authorization', () => {
|
|||
expect(result).toEqual(['web_search']);
|
||||
});
|
||||
|
||||
test('should not preserve malformed existing tools when registry is unavailable', async () => {
|
||||
test('should not preserve a tool key with no delimiter at all when registry is unavailable', async () => {
|
||||
// A key that isn't a real MCP tool key (no delimiter, so it has no
|
||||
// resolvable server) is rejected regardless of the existing-tools
|
||||
// fallback - unlike a key with multiple delimiters, which does have a
|
||||
// resolvable server (the segment after the last delimiter) and is
|
||||
// covered separately below.
|
||||
getMCPServersRegistry.mockImplementation(() => {
|
||||
throw new Error('MCPServersRegistry has not been initialized.');
|
||||
});
|
||||
|
||||
const malformedTool = `a${d}b${d}c`;
|
||||
// Deliberately not named anything containing "_mcp_" - that would
|
||||
// ironically make it an MCP tool key itself, exactly the class of
|
||||
// naming collision this whole regression is about. (Confirmed
|
||||
// programmatically, not just by eye - it's an easy mistake to repeat.)
|
||||
const noDelimiterTool = 'regular_web_tool';
|
||||
const result = await filterAuthorizedTools({
|
||||
tools: [malformedTool, `legit${d}serverA`, 'web_search'],
|
||||
tools: [noDelimiterTool, `legit${d}serverA`, 'web_search'],
|
||||
userId,
|
||||
user: testUser,
|
||||
availableTools,
|
||||
existingTools: [malformedTool, `legit${d}serverA`],
|
||||
existingTools: [noDelimiterTool, `legit${d}serverA`],
|
||||
});
|
||||
|
||||
expect(result).toContain(`legit${d}serverA`);
|
||||
expect(result).toContain('web_search');
|
||||
expect(result).not.toContain(malformedTool);
|
||||
expect(result).not.toContain(noDelimiterTool);
|
||||
});
|
||||
|
||||
test('should preserve an existing MCP tool key with multiple delimiters when registry is unavailable', async () => {
|
||||
// Regression test for https://github.com/danny-avila/LibreChat/issues/14440:
|
||||
// a tool key with more than one delimiter occurrence is not inherently
|
||||
// malformed - it just means the raw tool-name half (everything before
|
||||
// the *last* delimiter) itself contains the delimiter substring, which
|
||||
// legitimately happens with some upstream MCP tool names. The
|
||||
// registry-unavailable fallback should treat it like any other
|
||||
// previously-persisted tool, not single it out as broken.
|
||||
getMCPServersRegistry.mockImplementation(() => {
|
||||
throw new Error('MCPServersRegistry has not been initialized.');
|
||||
});
|
||||
|
||||
const multiDelimiterTool = `a${d}b${d}c`;
|
||||
const result = await filterAuthorizedTools({
|
||||
tools: [multiDelimiterTool, `legit${d}serverA`, 'web_search'],
|
||||
userId,
|
||||
user: testUser,
|
||||
availableTools,
|
||||
existingTools: [multiDelimiterTool, `legit${d}serverA`],
|
||||
});
|
||||
|
||||
expect(result).toContain(multiDelimiterTool);
|
||||
expect(result).toContain(`legit${d}serverA`);
|
||||
expect(result).toContain('web_search');
|
||||
});
|
||||
|
||||
test('should gate app-level MCP tools present in the global tool cache', async () => {
|
||||
|
|
@ -398,12 +433,29 @@ describe('MCP Tool Authorization', () => {
|
|||
expect(mockGetAllServerConfigs).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('should reject malformed MCP tool keys with multiple delimiters', async () => {
|
||||
test('should resolve MCP tool keys with multiple delimiters using the last segment as the server name', async () => {
|
||||
// Regression test for https://github.com/danny-avila/LibreChat/issues/14440.
|
||||
// A tool key with more than one delimiter occurrence is not inherently
|
||||
// malformed - it means the raw tool-name half (the part before the
|
||||
// *last* delimiter, which is always the segment LibreChat itself
|
||||
// appends) legitimately contains the delimiter substring. Previously
|
||||
// any key with >2 segments was rejected outright; now the server name
|
||||
// is always the last segment, matching how the key is actually built.
|
||||
//
|
||||
// `multiSegmentTool` below has an unrelated string ("victimServer")
|
||||
// embedded in its raw-tool-name half purely to prove there's no way to
|
||||
// spoof a *different* server via that embedded text - only the real
|
||||
// last segment ("authorizedServer") is ever consulted for
|
||||
// authorization, so this does not grant access to anything the user
|
||||
// isn't already allowed to use.
|
||||
const multiSegmentTool = `attack${d}victimServer${d}authorizedServer`;
|
||||
const unauthorizedMultiSegmentTool = `a${d}b${d}c${d}forbiddenServer`;
|
||||
|
||||
const result = await filterAuthorizedTools({
|
||||
tools: [
|
||||
`attack${d}victimServer${d}authorizedServer`,
|
||||
multiSegmentTool,
|
||||
`legit${d}authorizedServer`,
|
||||
`a${d}b${d}c${d}d`,
|
||||
unauthorizedMultiSegmentTool,
|
||||
'web_search',
|
||||
],
|
||||
userId,
|
||||
|
|
@ -411,9 +463,14 @@ describe('MCP Tool Authorization', () => {
|
|||
availableTools,
|
||||
});
|
||||
|
||||
expect(result).toEqual([`legit${d}authorizedServer`, 'web_search']);
|
||||
expect(result).not.toContainEqual(expect.stringContaining('victimServer'));
|
||||
expect(result).not.toContainEqual(expect.stringContaining(`a${d}b`));
|
||||
expect(result).toContain(multiSegmentTool);
|
||||
expect(result).toContain(`legit${d}authorizedServer`);
|
||||
expect(result).toContain('web_search');
|
||||
// The unrelated embedded text does not let the key resolve to a
|
||||
// different, unauthorized server: only the true last segment
|
||||
// ("forbiddenServer", not in the mocked server configs) is checked,
|
||||
// and it's correctly rejected.
|
||||
expect(result).not.toContain(unauthorizedMultiSegmentTool);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -691,6 +748,62 @@ describe('MCP Tool Authorization', () => {
|
|||
expect(updatedAgent.tools).toContain(`newTool${d}anotherServer`);
|
||||
});
|
||||
|
||||
test('should drop mcpServerNames for a server detached in the same edit that adds another', async () => {
|
||||
mockReq.user.id = existingAgentAuthorId.toString();
|
||||
mockReq.params.id = existingAgentId;
|
||||
/** Swapping servers in one edit: authorizedServer loses its only tool while
|
||||
* anotherServer gains one. Carrying the prior names forward wholesale would
|
||||
* leave authorizedServer indexed, so its viewers would keep agent-scoped
|
||||
* access to a server the agent no longer references. */
|
||||
mockReq.body = { tools: ['web_search', `newTool${d}anotherServer`] };
|
||||
|
||||
await updateAgentHandler(mockReq, mockRes);
|
||||
|
||||
const agentInDb = await Agent.findOne({ id: existingAgentId });
|
||||
expect(agentInDb.tools).not.toContain(`existingTool${d}authorizedServer`);
|
||||
expect(agentInDb.tools).toContain(`newTool${d}anotherServer`);
|
||||
expect(agentInDb.mcpServerNames).toEqual(['anotherServer']);
|
||||
});
|
||||
|
||||
test('should preserve resolved mcpServerNames when a non-owner retains MCP tools', async () => {
|
||||
/** The shared-agent path keeps the existing MCP tools verbatim; re-deriving the
|
||||
* index from their keys would turn a delimiter-bearing configured server into
|
||||
* its trailing segment, which `ServerConfigsDB` then treats as a DB server. */
|
||||
await Agent.updateOne(
|
||||
{ id: existingAgentId },
|
||||
{
|
||||
tools: ['web_search', `existingTool${d}Google${d}Workspace`],
|
||||
mcpServerNames: [`Google${d}Workspace`],
|
||||
},
|
||||
);
|
||||
mockUserCanUseMCPServers.mockResolvedValue(false);
|
||||
mockReq.user.id = new mongoose.Types.ObjectId().toString();
|
||||
mockReq.params.id = existingAgentId;
|
||||
mockReq.body = { tools: ['web_search', `existingTool${d}Google${d}Workspace`] };
|
||||
|
||||
await updateAgentHandler(mockReq, mockRes);
|
||||
|
||||
const agentInDb = await Agent.findOne({ id: existingAgentId });
|
||||
expect(agentInDb.mcpServerNames).toEqual([`Google${d}Workspace`]);
|
||||
expect(agentInDb.mcpServerNames).not.toContain('Workspace');
|
||||
});
|
||||
|
||||
test('should let persistence derive when an unindexed agent retains MCP tools', async () => {
|
||||
/** A legacy or partially migrated agent can hold MCP tools with no stored
|
||||
* mcpServerNames. Pinning the index to [] here would suppress the derivation
|
||||
* in updateAgent and strip agent-scoped access to its DB-backed server. */
|
||||
await Agent.updateOne({ id: existingAgentId }, { $unset: { mcpServerNames: 1 } });
|
||||
mockReq.user.id = existingAgentAuthorId.toString();
|
||||
mockReq.params.id = existingAgentId;
|
||||
mockReq.body = { tools: ['web_search', `existingTool${d}authorizedServer`] };
|
||||
|
||||
await updateAgentHandler(mockReq, mockRes);
|
||||
|
||||
const agentInDb = await Agent.findOne({ id: existingAgentId });
|
||||
expect(agentInDb.tools).toContain(`existingTool${d}authorizedServer`);
|
||||
expect(agentInDb.mcpServerNames).toEqual(['authorizedServer']);
|
||||
});
|
||||
|
||||
test('should not query MCP registry when no new MCP tools added', async () => {
|
||||
mockReq.user.id = existingAgentAuthorId.toString();
|
||||
mockReq.params.id = existingAgentId;
|
||||
|
|
|
|||
|
|
@ -4,6 +4,8 @@ const { nanoid } = require('nanoid');
|
|||
const { logger } = require('@librechat/data-schemas');
|
||||
const {
|
||||
refreshS3Url,
|
||||
splitMCPToolKey,
|
||||
normalizeServerName,
|
||||
agentCreateSchema,
|
||||
agentUpdateSchema,
|
||||
refreshListAvatars,
|
||||
|
|
@ -227,9 +229,12 @@ const filterAuthorizedTools = async ({
|
|||
availableTools,
|
||||
existingTools,
|
||||
configServers,
|
||||
resolvedServerNames,
|
||||
}) => {
|
||||
const filteredTools = [];
|
||||
let mcpServerConfigs;
|
||||
/** normalized server name -> the raw key `mcpServerConfigs` is indexed by */
|
||||
let configNamesByNormalized = new Map();
|
||||
let registryUnavailable = false;
|
||||
const existingToolSet = existingTools?.length ? new Set(existingTools) : null;
|
||||
const hasMCPTools = tools.some((tool) => tool?.includes(Constants.mcp_delimiter));
|
||||
|
|
@ -273,10 +278,18 @@ const filterAuthorizedTools = async ({
|
|||
mcpServerConfigs = {};
|
||||
registryUnavailable = true;
|
||||
}
|
||||
configNamesByNormalized = new Map(
|
||||
Object.keys(mcpServerConfigs).map((name) => [normalizeServerName(name), name]),
|
||||
);
|
||||
}
|
||||
|
||||
const parts = tool.split(Constants.mcp_delimiter);
|
||||
if (parts.length !== 2) {
|
||||
/** Tool keys embed the normalized server name; the config is keyed by the raw name. */
|
||||
const [, normalizedServerName] = splitMCPToolKey(
|
||||
tool,
|
||||
Array.from(configNamesByNormalized.keys()),
|
||||
);
|
||||
const serverName = configNamesByNormalized.get(normalizedServerName) ?? normalizedServerName;
|
||||
if (!serverName) {
|
||||
logger.warn(
|
||||
`[filterAuthorizedTools] Rejected malformed MCP tool key "${tool}" for user ${userId}`,
|
||||
);
|
||||
|
|
@ -288,14 +301,14 @@ const filterAuthorizedTools = async ({
|
|||
continue;
|
||||
}
|
||||
|
||||
const [, serverName] = parts;
|
||||
if (!serverName || !Object.hasOwn(mcpServerConfigs, serverName)) {
|
||||
if (!Object.hasOwn(mcpServerConfigs, serverName)) {
|
||||
logger.warn(
|
||||
`[filterAuthorizedTools] Rejected MCP tool "${tool}" — server "${serverName}" not accessible to user ${userId}`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
resolvedServerNames?.add(serverName);
|
||||
filteredTools.push(tool);
|
||||
}
|
||||
|
||||
|
|
@ -458,6 +471,9 @@ const createAgentHandler = async (req, res) => {
|
|||
hasMCPTools ? resolveConfigServers(req) : Promise.resolve(undefined),
|
||||
]);
|
||||
const mcpPermissionContext = createMCPPermissionContext(req);
|
||||
/** Resolved during authorization, so persistence indexes the real server rather
|
||||
* than a suffix guess - see the note on `filterAuthorizedTools`. */
|
||||
const resolvedServerNames = new Set();
|
||||
agentData.tools = await filterAuthorizedTools({
|
||||
tools,
|
||||
userId,
|
||||
|
|
@ -466,7 +482,11 @@ const createAgentHandler = async (req, res) => {
|
|||
mcpPermissionContext,
|
||||
availableTools,
|
||||
configServers,
|
||||
resolvedServerNames,
|
||||
});
|
||||
if (hasMCPTools) {
|
||||
agentData.mcpServerNames = Array.from(resolvedServerNames);
|
||||
}
|
||||
|
||||
const agent = await db.createAgent(agentData);
|
||||
|
||||
|
|
@ -752,6 +772,8 @@ const updateAgentHandler = async (req, res) => {
|
|||
if (!(await mcpPermissionContext.canUseServers(req.user))) {
|
||||
if (editingOwnAgent) {
|
||||
updateData.tools = effectiveTools.filter((t) => !isMCPTool(t));
|
||||
/** Every MCP tool just went away, so nothing should stay indexed. */
|
||||
updateData.mcpServerNames = [];
|
||||
} else if (hasToolUpdate) {
|
||||
const existingMCPToolSet = new Set(existingMCPTools);
|
||||
const nextTools = updateData.tools.filter(
|
||||
|
|
@ -764,10 +786,19 @@ const updateAgentHandler = async (req, res) => {
|
|||
}
|
||||
}
|
||||
updateData.tools = nextTools;
|
||||
/** The agent's MCP tools are retained verbatim here, so carry its resolved
|
||||
* names across too. Left unset when the agent has none stored, so
|
||||
* `updateAgent` can still derive rather than being pinned to an empty
|
||||
* index that would strip agent-scoped access. */
|
||||
if (existingAgent.mcpServerNames?.length) {
|
||||
updateData.mcpServerNames = existingAgent.mcpServerNames;
|
||||
}
|
||||
}
|
||||
} else if (hasToolUpdate) {
|
||||
const existingToolSet = new Set(existingTools);
|
||||
const newMCPTools = requestedMCPTools.filter((t) => !existingToolSet.has(t));
|
||||
/** Names resolved during authorization of the newly added tools. */
|
||||
const resolvedServerNames = new Set();
|
||||
|
||||
if (newMCPTools.length > 0) {
|
||||
const [availableTools, configServers] = await Promise.all([
|
||||
|
|
@ -782,12 +813,38 @@ const updateAgentHandler = async (req, res) => {
|
|||
mcpPermissionContext,
|
||||
availableTools,
|
||||
configServers,
|
||||
resolvedServerNames,
|
||||
});
|
||||
const rejectedSet = new Set(newMCPTools.filter((t) => !approvedNew.includes(t)));
|
||||
if (rejectedSet.size > 0) {
|
||||
updateData.tools = updateData.tools.filter((t) => !rejectedSet.has(t));
|
||||
}
|
||||
}
|
||||
|
||||
/** Rebuild the index from the tools that survive this edit: carry a prior name
|
||||
* forward only while some retained tool still resolves to it, so detaching every
|
||||
* tool for a server revokes agent-scoped access to it. The agent's own persisted
|
||||
* names are the candidate set, which needs neither a registry query nor a guess. */
|
||||
const priorNames = existingAgent.mcpServerNames ?? [];
|
||||
if (priorNames.length > 0) {
|
||||
const priorNameSet = new Set(priorNames);
|
||||
for (const tool of updateData.tools ?? []) {
|
||||
if (typeof tool !== 'string' || !tool.includes(Constants.mcp_delimiter)) {
|
||||
continue;
|
||||
}
|
||||
const [, retainedName] = splitMCPToolKey(tool, priorNames);
|
||||
if (retainedName && priorNameSet.has(retainedName)) {
|
||||
resolvedServerNames.add(retainedName);
|
||||
}
|
||||
}
|
||||
}
|
||||
/** Supplying `[]` would pin the index empty and suppress `updateAgent`'s
|
||||
* derivation, so only assert it when the result is authoritative: either we
|
||||
* resolved names, or no MCP tool survives and the index genuinely is empty. */
|
||||
const retainsMCPTools = (updateData.tools ?? []).some(isMCPTool);
|
||||
if (resolvedServerNames.size > 0 || !retainsMCPTools) {
|
||||
updateData.mcpServerNames = Array.from(resolvedServerNames);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -965,6 +1022,9 @@ const duplicateAgentHandler = async (req, res) => {
|
|||
resolveConfigServers(req),
|
||||
]);
|
||||
const mcpPermissionContext = createMCPPermissionContext(req);
|
||||
/** The duplicate carries the source agent's `mcpServerNames`; replace it with what
|
||||
* this user is actually authorized for, or the copy would grant the source's servers. */
|
||||
const resolvedServerNames = new Set();
|
||||
newAgentData.tools = await filterAuthorizedTools({
|
||||
tools: newAgentData.tools,
|
||||
userId,
|
||||
|
|
@ -974,7 +1034,25 @@ const duplicateAgentHandler = async (req, res) => {
|
|||
availableTools,
|
||||
existingTools: newAgentData.tools,
|
||||
configServers,
|
||||
resolvedServerNames,
|
||||
});
|
||||
/** When the registry is unavailable, `filterAuthorizedTools` grandfathers the
|
||||
* source's tools without resolving them, so carry forward the source names those
|
||||
* retained tools still point at rather than blanking the index. */
|
||||
const sourceNames = agent.mcpServerNames ?? [];
|
||||
if (sourceNames.length > 0) {
|
||||
const sourceNameSet = new Set(sourceNames);
|
||||
for (const tool of newAgentData.tools ?? []) {
|
||||
if (typeof tool !== 'string' || !tool.includes(Constants.mcp_delimiter)) {
|
||||
continue;
|
||||
}
|
||||
const [, retainedName] = splitMCPToolKey(tool, sourceNames);
|
||||
if (retainedName && sourceNameSet.has(retainedName)) {
|
||||
resolvedServerNames.add(retainedName);
|
||||
}
|
||||
}
|
||||
}
|
||||
newAgentData.mcpServerNames = Array.from(resolvedServerNames);
|
||||
}
|
||||
|
||||
if (newAgentData.tool_resources) {
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ const {
|
|||
checkAccess,
|
||||
isUserSourced,
|
||||
MCPErrorCodes,
|
||||
splitMCPToolKey,
|
||||
redactServerSecrets,
|
||||
redactAllServerSecrets,
|
||||
isMCPDomainNotAllowedError,
|
||||
|
|
@ -177,7 +178,7 @@ const getMCPTools = async (req, res) => {
|
|||
continue;
|
||||
}
|
||||
|
||||
const toolName = toolKey.split(Constants.mcp_delimiter)[0];
|
||||
const [toolName] = splitMCPToolKey(toolKey, [serverName]);
|
||||
server.tools.push({
|
||||
name: toolName,
|
||||
pluginKey: toolKey,
|
||||
|
|
|
|||
|
|
@ -6,7 +6,9 @@ const {
|
|||
PENDING_STALE_MS,
|
||||
MCPOAuthHandler,
|
||||
isMCPDomainAllowed,
|
||||
splitMCPToolKey,
|
||||
normalizeServerName,
|
||||
resolveMCPServerContext,
|
||||
normalizeJsonSchema,
|
||||
GenerationJobManager,
|
||||
resolveJsonSchemaRefs,
|
||||
|
|
@ -143,6 +145,50 @@ async function resolveMcpConfigNames(req) {
|
|||
return Object.keys(appConfig?.mcpConfig || {});
|
||||
}
|
||||
|
||||
/**
|
||||
* All configured server names in the normalized form tool keys are built with.
|
||||
* Unlike `resolveConfigServers`, this keeps unmodified YAML servers, which
|
||||
* `ensureConfigServers` skips - those are exactly the ones that must still
|
||||
* resolve the tool-key boundary.
|
||||
* @param {import('express').Request} req
|
||||
* @returns {Promise<string[]>}
|
||||
*/
|
||||
async function resolveMcpServerNames(req) {
|
||||
try {
|
||||
const names = await resolveMcpConfigNames(req);
|
||||
return names.map(normalizeServerName);
|
||||
} catch (error) {
|
||||
logger.warn(
|
||||
'[resolveMcpServerNames] Failed to resolve server names, degrading to empty:',
|
||||
error,
|
||||
);
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Config-source servers and all configured names from a single app-config read,
|
||||
* so the tool-loading path does not pay two lookups for the same principal.
|
||||
* Degrades to empty like `resolveConfigServers` rather than aborting tool loading.
|
||||
* @param {import('express').Request} req
|
||||
* @returns {Promise<{ configServers: Record<string, import('@librechat/api').ParsedServerConfig>, serverNames: string[] }>}
|
||||
*/
|
||||
async function resolveMcpServerContext(req) {
|
||||
try {
|
||||
const appConfig = await getAppConfigForRequest(req);
|
||||
return await resolveMCPServerContext({
|
||||
mcpConfig: appConfig?.mcpConfig || {},
|
||||
ensureConfigServers: (mcpConfig) => getMCPServersRegistry().ensureConfigServers(mcpConfig),
|
||||
});
|
||||
} catch (error) {
|
||||
logger.warn(
|
||||
'[resolveMcpServerContext] Failed to resolve MCP servers, degrading to empty:',
|
||||
error,
|
||||
);
|
||||
return { configServers: {}, serverNames: [] };
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves config-source servers and merges all server configs (YAML + config + user DB)
|
||||
* for the given user context. Shared helper for controllers needing the full merged config.
|
||||
|
|
@ -613,6 +659,7 @@ async function createMCPTools({
|
|||
streamId,
|
||||
jobCreatedAt,
|
||||
availableTools: result.availableTools,
|
||||
serverName,
|
||||
toolKey: `${tool.name}${Constants.mcp_delimiter}${serverName}`,
|
||||
requestBody,
|
||||
requestScopedConnections,
|
||||
|
|
@ -661,11 +708,22 @@ async function createMCPTool({
|
|||
requestScopedConnections,
|
||||
config,
|
||||
configServers,
|
||||
serverName: resolvedServerName,
|
||||
onAvailableTools,
|
||||
streamId = null,
|
||||
jobCreatedAt,
|
||||
}) {
|
||||
const [toolName, serverName] = toolKey.split(Constants.mcp_delimiter);
|
||||
/** `loadTools` already resolved the server for this key; parsing is the fallback. */
|
||||
const [parsedToolName, parsedServerName] = splitMCPToolKey(
|
||||
toolKey,
|
||||
/** Tool keys embed the normalized server name, so the candidate list must be
|
||||
* normalized too or a name needing normalization never matches. */
|
||||
resolvedServerName
|
||||
? [normalizeServerName(resolvedServerName)]
|
||||
: Object.keys(configServers ?? {}).map(normalizeServerName),
|
||||
);
|
||||
const serverName = resolvedServerName ?? parsedServerName;
|
||||
const toolName = parsedToolName;
|
||||
|
||||
const serverConfig =
|
||||
config ?? (await getMCPServersRegistry().getServerConfig(serverName, user?.id, configServers));
|
||||
|
|
@ -1094,6 +1152,8 @@ module.exports = {
|
|||
userCanUseMCPServers,
|
||||
getMCPSetupData,
|
||||
resolveConfigServers,
|
||||
resolveMcpServerNames,
|
||||
resolveMcpServerContext,
|
||||
resolveMcpConfigNames,
|
||||
resolveAllMcpConfigs,
|
||||
createOAuthStart,
|
||||
|
|
|
|||
|
|
@ -69,7 +69,7 @@ const { primeFiles: primeCodeFiles } = require('~/server/services/Files/Code/pro
|
|||
const { manifestToolMap, toolkits } = require('~/app/clients/tools/manifest');
|
||||
const { createOnSearchResults } = require('~/server/services/Tools/search');
|
||||
const { reinitMCPServer } = require('~/server/services/Tools/mcp');
|
||||
const { createMCPPermissionContext, resolveConfigServers } = require('~/server/services/MCP');
|
||||
const { createMCPPermissionContext, resolveMcpServerContext } = require('~/server/services/MCP');
|
||||
const { getMCPRequestContext } = require('~/server/services/MCPRequestContext');
|
||||
const { recordUsage } = require('~/server/services/Threads');
|
||||
const { loadTools } = require('~/app/clients/tools/util');
|
||||
|
|
@ -607,19 +607,26 @@ async function loadToolDefinitionsWrapper({
|
|||
return { toolDefinitions: [] };
|
||||
}
|
||||
|
||||
/** Only MCP tool keys need the server context; a purely non-MCP agent should not
|
||||
* pay an app-config lookup on startup. */
|
||||
const hasFilteredMCPTools = filteredTools.some((t) => t.includes(Constants.mcp_delimiter));
|
||||
const { configServers, serverNames: mcpServerNames } = hasFilteredMCPTools
|
||||
? await resolveMcpServerContext(req)
|
||||
: { configServers: {}, serverNames: [] };
|
||||
|
||||
/** @type {Record<string, Record<string, string>>} */
|
||||
let userMCPAuthMap;
|
||||
if (filteredTools?.some((t) => t.includes(Constants.mcp_delimiter))) {
|
||||
if (hasFilteredMCPTools) {
|
||||
userMCPAuthMap = await getUserMCPAuthMap({
|
||||
tools: filteredTools,
|
||||
userId: req.user.id,
|
||||
serverNames: mcpServerNames,
|
||||
findPluginAuthsByKeys,
|
||||
});
|
||||
}
|
||||
|
||||
const flowsCache = getLogStores(CacheKeys.FLOWS);
|
||||
const flowManager = getFlowStateManager(flowsCache);
|
||||
const configServers = await resolveConfigServers(req);
|
||||
const pendingOAuthServers = new Set();
|
||||
const pendingOAuthStarts = new Map();
|
||||
const emittedOAuthStarts = new Map();
|
||||
|
|
@ -885,6 +892,7 @@ async function loadToolDefinitionsWrapper({
|
|||
programmaticToolsEnabled,
|
||||
codeExecutionEnabled,
|
||||
provider: agent.provider,
|
||||
mcpServerNames,
|
||||
},
|
||||
{
|
||||
isBuiltInTool,
|
||||
|
|
@ -893,7 +901,7 @@ async function loadToolDefinitionsWrapper({
|
|||
},
|
||||
);
|
||||
|
||||
for (const serverName of getMCPServerNamesFromTools(filteredTools)) {
|
||||
for (const serverName of getMCPServerNamesFromTools(filteredTools, mcpServerNames)) {
|
||||
if (pendingOAuthServers.has(serverName)) {
|
||||
continue;
|
||||
}
|
||||
|
|
@ -967,6 +975,7 @@ async function loadToolDefinitionsWrapper({
|
|||
programmaticToolsEnabled,
|
||||
codeExecutionEnabled,
|
||||
provider: agent.provider,
|
||||
mcpServerNames,
|
||||
},
|
||||
{
|
||||
isBuiltInTool,
|
||||
|
|
@ -1181,12 +1190,18 @@ async function loadAgentTools({
|
|||
webSearchCallbacks = createOnSearchResults(res, streamId, jobCreatedAt);
|
||||
}
|
||||
|
||||
/** Resolved once and threaded into `loadTools` so the request app config is read once. */
|
||||
const mcpServerContext = _agentTools?.some((t) => t.includes(Constants.mcp_delimiter))
|
||||
? await resolveMcpServerContext(req)
|
||||
: undefined;
|
||||
|
||||
/** @type {Record<string, Record<string, string>>} */
|
||||
let userMCPAuthMap;
|
||||
if (_agentTools?.some((t) => t.includes(Constants.mcp_delimiter))) {
|
||||
if (mcpServerContext) {
|
||||
userMCPAuthMap = await getUserMCPAuthMap({
|
||||
tools: _agentTools,
|
||||
userId: req.user.id,
|
||||
serverNames: mcpServerContext.serverNames,
|
||||
findPluginAuthsByKeys,
|
||||
});
|
||||
}
|
||||
|
|
@ -1201,6 +1216,7 @@ async function loadAgentTools({
|
|||
options: {
|
||||
req,
|
||||
res,
|
||||
mcpServerContext,
|
||||
jobCreatedAt,
|
||||
openAIApiKey,
|
||||
tool_resources,
|
||||
|
|
|
|||
|
|
@ -32,6 +32,13 @@ jest.mock('@librechat/api', () => ({
|
|||
GenerationJobManager: jest.fn(),
|
||||
resolveJsonSchemaRefs: jest.fn((schema) => schema),
|
||||
buildOAuthToolCallName: jest.fn((name) => name),
|
||||
/** Mirrors the real resolver so these tests still exercise the wrapper's own
|
||||
* plumbing - loading the request config and degrading on failure - rather than
|
||||
* the resolution logic, which is unit-tested in packages/api. */
|
||||
resolveMCPServerContext: jest.fn(async ({ mcpConfig, ensureConfigServers }) => ({
|
||||
configServers: await ensureConfigServers(mcpConfig),
|
||||
serverNames: Object.keys(mcpConfig),
|
||||
})),
|
||||
}));
|
||||
|
||||
jest.mock('~/cache', () => ({ getLogStores: jest.fn() }));
|
||||
|
|
@ -54,7 +61,12 @@ jest.mock('~/server/services/Tools/mcp', () => ({
|
|||
}));
|
||||
|
||||
const { getAppConfig } = require('~/server/services/Config');
|
||||
const { resolveConfigServers, resolveMcpConfigNames, resolveAllMcpConfigs } = require('../MCP');
|
||||
const {
|
||||
resolveConfigServers,
|
||||
resolveMcpConfigNames,
|
||||
resolveAllMcpConfigs,
|
||||
resolveMcpServerContext,
|
||||
} = require('../MCP');
|
||||
|
||||
describe('resolveConfigServers', () => {
|
||||
beforeEach(() => jest.clearAllMocks());
|
||||
|
|
@ -99,6 +111,42 @@ describe('resolveConfigServers', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('resolveMcpServerContext', () => {
|
||||
beforeEach(() => jest.clearAllMocks());
|
||||
|
||||
it('derives config servers and all configured names from a single app-config read', async () => {
|
||||
/** `ensureConfigServers` intentionally omits unmodified YAML servers, so the name
|
||||
* list must come from `mcpConfig` itself or boundary resolution goes inert. */
|
||||
getAppConfig.mockResolvedValue({ mcpConfig: { unchangedYaml: {}, lazyInit: {} } });
|
||||
mockRegistry.ensureConfigServers.mockResolvedValue({ lazyInit: { name: 'lazyInit' } });
|
||||
|
||||
const result = await resolveMcpServerContext({ user: { id: 'u1' } });
|
||||
|
||||
expect(result.configServers).toEqual({ lazyInit: { name: 'lazyInit' } });
|
||||
expect(result.serverNames.sort()).toEqual(['lazyInit', 'unchangedYaml']);
|
||||
expect(getAppConfig).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('degrades to empty rather than rejecting when the config lookup fails', async () => {
|
||||
/** A rejection here would abort tool loading entirely, defeating the
|
||||
* catch-and-degrade the sibling resolver already provides. */
|
||||
getAppConfig.mockRejectedValue(new Error('db timeout'));
|
||||
|
||||
const result = await resolveMcpServerContext({ user: { id: 'u1' } });
|
||||
|
||||
expect(result).toEqual({ configServers: {}, serverNames: [] });
|
||||
});
|
||||
|
||||
it('degrades to empty when ensureConfigServers throws', async () => {
|
||||
getAppConfig.mockResolvedValue({ mcpConfig: { srv: {} } });
|
||||
mockRegistry.ensureConfigServers.mockRejectedValue(new Error('inspect failed'));
|
||||
|
||||
const result = await resolveMcpServerContext({ user: { id: 'u1' } });
|
||||
|
||||
expect(result).toEqual({ configServers: {}, serverNames: [] });
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveMcpConfigNames', () => {
|
||||
beforeEach(() => jest.clearAllMocks());
|
||||
|
||||
|
|
|
|||
|
|
@ -45,6 +45,7 @@ const mockCreateActionTool = jest.fn();
|
|||
const mockGetServerConfig = jest.fn();
|
||||
const mockFlowManager = { getFlowState: jest.fn() };
|
||||
const mockResolveConfigServers = jest.fn();
|
||||
const mockResolveMcpServerNames = jest.fn();
|
||||
const mockUserCanUseMCPServers = jest.fn().mockResolvedValue(true);
|
||||
jest.mock('~/server/services/Tools/credentials', () => ({
|
||||
loadAuthValues: jest.fn().mockResolvedValue({}),
|
||||
|
|
@ -86,6 +87,11 @@ jest.mock('~/config', () => ({
|
|||
}));
|
||||
jest.mock('~/server/services/MCP', () => ({
|
||||
resolveConfigServers: (...args) => mockResolveConfigServers(...args),
|
||||
resolveMcpServerNames: (...args) => mockResolveMcpServerNames(...args),
|
||||
resolveMcpServerContext: async (...args) => {
|
||||
const configServers = (await mockResolveConfigServers(...args)) ?? {};
|
||||
return { configServers, serverNames: Object.keys(configServers) };
|
||||
},
|
||||
createMCPPermissionContext: jest.fn((req) => ({
|
||||
canUseServers: (user) => mockUserCanUseMCPServers(user, req),
|
||||
})),
|
||||
|
|
@ -140,6 +146,7 @@ describe('ToolService - Action Capability Gating', () => {
|
|||
mockGetServerConfig.mockResolvedValue(undefined);
|
||||
mockFlowManager.getFlowState.mockResolvedValue(undefined);
|
||||
mockResolveConfigServers.mockResolvedValue({});
|
||||
mockResolveMcpServerNames.mockResolvedValue([]);
|
||||
});
|
||||
|
||||
describe('resolveAgentCapabilities', () => {
|
||||
|
|
@ -679,6 +686,9 @@ describe('ToolService - Action Capability Gating', () => {
|
|||
const mcpTool = `search${Constants.mcp_delimiter}${serverName}`;
|
||||
const capabilities = [AgentCapabilities.tools];
|
||||
const req = createMockReq(capabilities);
|
||||
/** A server whose own name contains the delimiter is only resolvable
|
||||
* against the configured set, so the key boundary is unambiguous. */
|
||||
mockResolveConfigServers.mockResolvedValue({ [serverName]: {} });
|
||||
const res = { writableEnded: false };
|
||||
mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities));
|
||||
mockFlowManager.getFlowState.mockResolvedValue({
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ import { MessageContext } from '~/Providers/MessageContext';
|
|||
import MessageIcon from '~/components/Share/MessageIcon';
|
||||
import { subagentProgressByToolCallId } from '~/store';
|
||||
import { useAgentsMapContext } from '~/Providers';
|
||||
import { useMCPServerNames } from '~/hooks/MCP';
|
||||
import { AttachmentGroup } from './Attachment';
|
||||
import { useLocalize } from '~/hooks';
|
||||
import Reasoning from './Reasoning';
|
||||
|
|
@ -704,11 +705,13 @@ function ToolNameBadge({ name }: { name: string }): JSX.Element {
|
|||
function ToolIdentifier({
|
||||
rawName,
|
||||
localize,
|
||||
mcpServerNames,
|
||||
}: {
|
||||
rawName: string;
|
||||
localize: ReturnType<typeof useLocalize>;
|
||||
mcpServerNames?: readonly string[];
|
||||
}): JSX.Element {
|
||||
const parsed = parseToolName(rawName);
|
||||
const parsed = parseToolName(rawName, mcpServerNames);
|
||||
if (parsed.mcpServer) {
|
||||
return (
|
||||
<span className="inline-flex min-w-0 shrink items-baseline gap-1">
|
||||
|
|
@ -740,6 +743,7 @@ function ToolIdentifier({
|
|||
*/
|
||||
function TickerLineView({ line }: { line: SubagentTickerLine }): JSX.Element {
|
||||
const localize = useLocalize();
|
||||
const mcpServerNames = useMCPServerNames();
|
||||
if (line.kind === 'writing' || line.kind === 'reasoning') {
|
||||
const prefix =
|
||||
line.kind === 'writing'
|
||||
|
|
@ -766,7 +770,7 @@ function TickerLineView({ line }: { line: SubagentTickerLine }): JSX.Element {
|
|||
{line.toolNames.map((name, i) => (
|
||||
<span key={`${i}-${name}`} className="flex min-w-0 items-baseline gap-1">
|
||||
{i > 0 && <span className="shrink-0 text-text-tertiary">,</span>}
|
||||
<ToolIdentifier rawName={name} localize={localize} />
|
||||
<ToolIdentifier rawName={name} localize={localize} mcpServerNames={mcpServerNames} />
|
||||
</span>
|
||||
))}
|
||||
{line.argsSnippet && (
|
||||
|
|
@ -779,7 +783,11 @@ function TickerLineView({ line }: { line: SubagentTickerLine }): JSX.Element {
|
|||
if (line.kind === 'tool_complete') {
|
||||
return (
|
||||
<li className="flex w-full items-baseline gap-1 overflow-hidden whitespace-nowrap">
|
||||
<ToolIdentifier rawName={line.toolName} localize={localize} />
|
||||
<ToolIdentifier
|
||||
rawName={line.toolName}
|
||||
localize={localize}
|
||||
mcpServerNames={mcpServerNames}
|
||||
/>
|
||||
<span className="shrink-0 text-text-tertiary">→</span>
|
||||
<span
|
||||
dir="rtl"
|
||||
|
|
|
|||
|
|
@ -125,6 +125,11 @@ jest.mock('~/components/Share/MessageIcon', () => ({
|
|||
),
|
||||
}));
|
||||
|
||||
jest.mock('~/hooks/MCP', () => {
|
||||
const mcpServerNames: string[] = [];
|
||||
return { useMCPServerNames: () => mcpServerNames };
|
||||
});
|
||||
|
||||
jest.mock('~/utils', () => ({
|
||||
...jest.requireActual('~/utils/groupToolCalls'),
|
||||
...jest.requireActual('~/utils/toolLabels'),
|
||||
|
|
|
|||
|
|
@ -7,11 +7,12 @@ import {
|
|||
dataService,
|
||||
actionDelimiter,
|
||||
actionDomainSeparator,
|
||||
splitToolCallName,
|
||||
} from 'librechat-data-provider';
|
||||
import type { TAttachment } from 'librechat-data-provider';
|
||||
import { useLocalize, useProgress, useExpandCollapse } from '~/hooks';
|
||||
import { ToolIcon, getToolIconType, isError } from './ToolOutput';
|
||||
import { useMCPIconMap } from '~/hooks/MCP';
|
||||
import { useMCPIconMap, useMCPServerNames } from '~/hooks/MCP';
|
||||
import { AttachmentGroup } from './Parts';
|
||||
import ToolCallInfo from './ToolCallInfo';
|
||||
import ProgressText from './ProgressText';
|
||||
|
|
@ -66,14 +67,13 @@ export default function ToolCall({
|
|||
}
|
||||
}, [auth]);
|
||||
|
||||
const mcpServerNames = useMCPServerNames();
|
||||
const { function_name, domain, isMCPToolCall, mcpServerName } = useMemo(() => {
|
||||
if (typeof name !== 'string') {
|
||||
return { function_name: '', domain: null, isMCPToolCall: false, mcpServerName: '' };
|
||||
}
|
||||
if (name.includes(Constants.mcp_delimiter)) {
|
||||
const parts = name.split(Constants.mcp_delimiter);
|
||||
const func = parts[0];
|
||||
const server = parts.slice(1).join(Constants.mcp_delimiter);
|
||||
const [func, server = ''] = splitToolCallName(name, mcpServerNames);
|
||||
const displayName = func === 'oauth' ? server : func;
|
||||
return {
|
||||
function_name: displayName || '',
|
||||
|
|
@ -105,7 +105,7 @@ export default function ToolCall({
|
|||
isMCPToolCall: false,
|
||||
mcpServerName: '',
|
||||
};
|
||||
}, [name, parsedAuthUrl]);
|
||||
}, [name, parsedAuthUrl, mcpServerNames]);
|
||||
|
||||
const toolIconType = useMemo(() => getToolIconType(name), [name]);
|
||||
const mcpIconMap = useMCPIconMap();
|
||||
|
|
|
|||
|
|
@ -10,11 +10,11 @@ import type {
|
|||
} from 'librechat-data-provider';
|
||||
import type { PartWithIndex } from './ParallelContent';
|
||||
import { useLocalize, useExpandCollapse, scheduleMessageContentLayoutReconcile } from '~/hooks';
|
||||
import { useMCPIconMap, useMCPServerNames } from '~/hooks/MCP';
|
||||
import { isBashProgrammaticToolCall } from './routing';
|
||||
import { ASK_USER_QUESTION } from '~/utils/approval';
|
||||
import { cn, getToolDisplayLabel } from '~/utils';
|
||||
import { StackedToolIcons } from './ToolOutput';
|
||||
import { useMCPIconMap } from '~/hooks/MCP';
|
||||
import { AttachmentGroup } from './Parts';
|
||||
import store from '~/store';
|
||||
|
||||
|
|
@ -126,6 +126,7 @@ export default function ToolCallGroup({
|
|||
}: ToolCallGroupProps) {
|
||||
const localize = useLocalize();
|
||||
const mcpIconMap = useMCPIconMap();
|
||||
const mcpServerNames = useMCPServerNames();
|
||||
const rootRef = useRef<HTMLDivElement | null>(null);
|
||||
const cancelLayoutReconcileRef = useRef<(() => void) | null>(null);
|
||||
const retainedForPendingApprovalRef = useRef(false);
|
||||
|
|
@ -179,7 +180,7 @@ export default function ToolCallGroup({
|
|||
const labels: string[] = [];
|
||||
for (const rawName of toolNames) {
|
||||
if (!rawName) continue;
|
||||
const label = getToolDisplayLabel(rawName, localize);
|
||||
const label = getToolDisplayLabel(rawName, localize, mcpServerNames);
|
||||
if (!seen.has(label)) {
|
||||
seen.add(label);
|
||||
labels.push(label);
|
||||
|
|
@ -189,7 +190,7 @@ export default function ToolCallGroup({
|
|||
return labels.join(', ');
|
||||
}
|
||||
return `${labels.slice(0, 3).join(', ')}, +${labels.length - 3}`;
|
||||
}, [toolNames, localize]);
|
||||
}, [toolNames, localize, mcpServerNames]);
|
||||
|
||||
const autoExpand = useRecoilValue(store.autoExpandTools);
|
||||
const autoCollapse = !autoExpand && count >= 2 && allCompleted;
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import { useMemo } from 'react';
|
||||
import ToolIcon, { getToolIconType, getMCPServerName } from './ToolIcon';
|
||||
import type { ToolIconType } from './ToolIcon';
|
||||
import ToolIcon, { getToolIconType, getMCPServerName } from './ToolIcon';
|
||||
import { useMCPServerNames } from '~/hooks/MCP';
|
||||
import { cn } from '~/utils';
|
||||
|
||||
interface ResolvedIcon {
|
||||
|
|
@ -22,12 +23,13 @@ export default function StackedToolIcons({
|
|||
maxIcons = 3,
|
||||
isAnimating = false,
|
||||
}: StackedToolIconsProps) {
|
||||
const mcpServerNames = useMCPServerNames();
|
||||
const uniqueIcons = useMemo(() => {
|
||||
const seen = new Set<string>();
|
||||
const result: ResolvedIcon[] = [];
|
||||
for (const name of toolNames) {
|
||||
const type = getToolIconType(name);
|
||||
const serverName = getMCPServerName(name);
|
||||
const serverName = getMCPServerName(name, mcpServerNames);
|
||||
const iconUrl = serverName ? mcpIconMap?.get(serverName) : undefined;
|
||||
const key = iconUrl ? `mcp-${serverName}` : type;
|
||||
if (!seen.has(key)) {
|
||||
|
|
@ -36,7 +38,7 @@ export default function StackedToolIcons({
|
|||
}
|
||||
}
|
||||
return result;
|
||||
}, [toolNames, mcpIconMap]);
|
||||
}, [toolNames, mcpIconMap, mcpServerNames]);
|
||||
|
||||
const visibleIcons = uniqueIcons.slice(0, maxIcons);
|
||||
const overflowCount = uniqueIcons.length - visibleIcons.length;
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { Constants, isActionTool } from 'librechat-data-provider';
|
||||
import { Constants, isActionTool, splitToolCallName } from 'librechat-data-provider';
|
||||
import {
|
||||
Terminal,
|
||||
Globe,
|
||||
|
|
@ -91,13 +91,12 @@ export function getToolIconType(name: string): ToolIconType {
|
|||
}
|
||||
|
||||
/** Extracts the MCP server name from a tool name with format `tool<delimiter>server`. */
|
||||
export function getMCPServerName(toolName: string): string {
|
||||
const idx = toolName.indexOf(Constants.mcp_delimiter);
|
||||
if (idx < 0) {
|
||||
export function getMCPServerName(toolName: string, knownServerNames?: readonly string[]): string {
|
||||
if (!toolName.includes(Constants.mcp_delimiter)) {
|
||||
return '';
|
||||
}
|
||||
const afterDelimiter = toolName.slice(idx + Constants.mcp_delimiter.length);
|
||||
return afterDelimiter || '';
|
||||
const [, serverName] = splitToolCallName(toolName, knownServerNames);
|
||||
return serverName ?? '';
|
||||
}
|
||||
|
||||
interface ToolIconProps {
|
||||
|
|
|
|||
|
|
@ -1,8 +1,8 @@
|
|||
import React from 'react';
|
||||
import { RecoilRoot } from 'recoil';
|
||||
import { ContentTypes } from 'librechat-data-provider';
|
||||
import type { TAttachment, TMessageContentParts } from 'librechat-data-provider';
|
||||
import { fireEvent, render, screen } from '@testing-library/react';
|
||||
import type { TAttachment, TMessageContentParts } from 'librechat-data-provider';
|
||||
import ContentParts from '../ContentParts';
|
||||
|
||||
jest.mock('~/hooks', () => ({
|
||||
|
|
@ -20,9 +20,13 @@ jest.mock('~/hooks', () => ({
|
|||
scheduleMessageContentLayoutReconcile: jest.fn(() => jest.fn()),
|
||||
}));
|
||||
|
||||
jest.mock('~/hooks/MCP', () => ({
|
||||
useMCPIconMap: () => new Map(),
|
||||
}));
|
||||
jest.mock('~/hooks/MCP', () => {
|
||||
const mcpServerNames: string[] = [];
|
||||
return {
|
||||
useMCPIconMap: () => new Map(),
|
||||
useMCPServerNames: () => mcpServerNames,
|
||||
};
|
||||
});
|
||||
|
||||
jest.mock('../ToolOutput', () => ({
|
||||
StackedToolIcons: () => <span data-testid="stacked-icons" />,
|
||||
|
|
|
|||
|
|
@ -33,9 +33,13 @@ jest.mock('~/hooks', () => ({
|
|||
}),
|
||||
}));
|
||||
|
||||
jest.mock('~/hooks/MCP', () => ({
|
||||
useMCPIconMap: () => new Map(),
|
||||
}));
|
||||
jest.mock('~/hooks/MCP', () => {
|
||||
const mcpServerNames: string[] = [];
|
||||
return {
|
||||
useMCPIconMap: () => new Map(),
|
||||
useMCPServerNames: () => mcpServerNames,
|
||||
};
|
||||
});
|
||||
|
||||
jest.mock('~/components/Chat/Messages/Content/MessageContent', () => ({
|
||||
__esModule: true,
|
||||
|
|
|
|||
|
|
@ -32,9 +32,13 @@ jest.mock('~/hooks', () => ({
|
|||
scheduleMessageContentLayoutReconcile: jest.fn(() => jest.fn()),
|
||||
}));
|
||||
|
||||
jest.mock('~/hooks/MCP', () => ({
|
||||
useMCPIconMap: () => new Map(),
|
||||
}));
|
||||
jest.mock('~/hooks/MCP', () => {
|
||||
const mcpServerNames: string[] = [];
|
||||
return {
|
||||
useMCPIconMap: () => new Map(),
|
||||
useMCPServerNames: () => mcpServerNames,
|
||||
};
|
||||
});
|
||||
|
||||
jest.mock('../ToolOutput', () => ({
|
||||
StackedToolIcons: ({ toolNames }: { toolNames: string[] }) => (
|
||||
|
|
|
|||
43
client/src/hooks/MCP/__tests__/useVisibleTools.test.ts
Normal file
43
client/src/hooks/MCP/__tests__/useVisibleTools.test.ts
Normal file
|
|
@ -0,0 +1,43 @@
|
|||
import { renderHook } from '@testing-library/react';
|
||||
import { Constants } from 'librechat-data-provider';
|
||||
import type { TPlugin } from 'librechat-data-provider';
|
||||
import type { MCPServerInfo } from '~/common';
|
||||
import { useVisibleTools } from '../useVisibleTools';
|
||||
|
||||
const d = Constants.mcp_delimiter;
|
||||
|
||||
describe('useVisibleTools', () => {
|
||||
const regularTools: TPlugin[] = [{ name: 'Web Search', pluginKey: 'web_search' }] as TPlugin[];
|
||||
const mcpServersMap = new Map<string, MCPServerInfo>([
|
||||
['gitlab', {} as MCPServerInfo],
|
||||
['myserver', {} as MCPServerInfo],
|
||||
]);
|
||||
|
||||
it('resolves a normal single-delimiter MCP tool id to its server name', () => {
|
||||
const { result } = renderHook(() =>
|
||||
useVisibleTools([`search${d}myserver`], regularTools, mcpServersMap),
|
||||
);
|
||||
expect(result.current.mcpServerNames).toEqual(['myserver']);
|
||||
expect(result.current.toolIds).toEqual([]);
|
||||
});
|
||||
|
||||
it('resolves an MCP tool id whose raw tool name itself contains the delimiter substring', () => {
|
||||
// Regression test for https://github.com/danny-avila/LibreChat/issues/14440:
|
||||
// a raw MCP tool name that already contains "_mcp_" (e.g. one exposed
|
||||
// through a gateway that prefixes tool names by server) must still
|
||||
// resolve to the real server name - the *last* segment, not
|
||||
// `.split(delimiter)[1]`, which would grab the wrong (middle) segment
|
||||
// once there's more than one occurrence.
|
||||
const toolId = `gitlab-get${d}server_version${d}gitlab`;
|
||||
const { result } = renderHook(() => useVisibleTools([toolId], regularTools, mcpServersMap));
|
||||
expect(result.current.mcpServerNames).toEqual(['gitlab']);
|
||||
});
|
||||
|
||||
it('keeps regular (non-MCP) tools separate from MCP server names', () => {
|
||||
const { result } = renderHook(() =>
|
||||
useVisibleTools(['web_search'], regularTools, mcpServersMap),
|
||||
);
|
||||
expect(result.current.toolIds).toEqual(['web_search']);
|
||||
expect(result.current.mcpServerNames).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
|
@ -3,5 +3,5 @@ export * from './useVisibleTools';
|
|||
export * from './useMCPServerManager';
|
||||
export * from './useMCPConnectionStatus';
|
||||
|
||||
export { useMCPIconMap } from './useMCPIconMap';
|
||||
export { useMCPIconMap, useMCPServerNames } from './useMCPIconMap';
|
||||
export { useRemoveMCPTool } from './useRemoveMCPTool';
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { useMemo } from 'react';
|
||||
import { normalizeServerName } from 'librechat-data-provider';
|
||||
import { useMCPServersQuery } from '~/data-provider';
|
||||
|
||||
export function useMCPIconMap(): Map<string, string> {
|
||||
|
|
@ -11,9 +12,20 @@ export function useMCPIconMap(): Map<string, string> {
|
|||
}
|
||||
for (const [serverName, config] of Object.entries(servers)) {
|
||||
if (config.iconPath) {
|
||||
map.set(serverName, config.iconPath);
|
||||
/** Looked up with a server name parsed out of a tool key, which carries the
|
||||
* normalized form, so key the map the same way. */
|
||||
map.set(normalizeServerName(serverName), config.iconPath);
|
||||
}
|
||||
}
|
||||
return map;
|
||||
}, [servers]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Configured MCP server names in the normalized form tool keys are built from,
|
||||
* so they can be matched against a key. The config is keyed by the raw name.
|
||||
*/
|
||||
export function useMCPServerNames(): string[] {
|
||||
const { data: servers } = useMCPServersQuery();
|
||||
return useMemo(() => (servers ? Object.keys(servers).map(normalizeServerName) : []), [servers]);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { useMemo } from 'react';
|
||||
import { Constants } from 'librechat-data-provider';
|
||||
import { Constants, splitMCPToolKey } from 'librechat-data-provider';
|
||||
import type { TPlugin } from 'librechat-data-provider';
|
||||
import type { MCPServerInfo } from '~/common';
|
||||
|
||||
|
|
@ -23,13 +23,14 @@ export function useVisibleTools(
|
|||
mcpServersMap: Map<string, MCPServerInfo>,
|
||||
): VisibleToolsResult {
|
||||
return useMemo(() => {
|
||||
const knownServerNames = Array.from(mcpServersMap.keys());
|
||||
const mcpServers = new Set<string>();
|
||||
const regularToolIds: string[] = [];
|
||||
|
||||
for (const toolId of selectedToolIds ?? []) {
|
||||
// MCP tools/servers
|
||||
if (toolId.includes(Constants.mcp_delimiter)) {
|
||||
const serverName = toolId.split(Constants.mcp_delimiter)[1];
|
||||
const [, serverName] = splitMCPToolKey(toolId, knownServerNames);
|
||||
if (serverName) {
|
||||
mcpServers.add(serverName);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { Constants } from 'librechat-data-provider';
|
||||
import { Constants, splitToolCallName } from 'librechat-data-provider';
|
||||
import type { TranslationKeys } from '~/hooks';
|
||||
|
||||
/**
|
||||
|
|
@ -46,11 +46,12 @@ export interface ParsedToolName {
|
|||
* - `web_search` → `{ mcpServer: '', toolName: 'web_search', friendlyKey: 'com_ui_tool_name_web_search' }`
|
||||
* - `some_custom_tool` → `{ mcpServer: '', toolName: 'some_custom_tool' }`
|
||||
*/
|
||||
export function parseToolName(rawName: string): ParsedToolName {
|
||||
const idx = rawName.indexOf(Constants.mcp_delimiter);
|
||||
if (idx >= 0) {
|
||||
const mcpServer = rawName.slice(idx + Constants.mcp_delimiter.length);
|
||||
const toolName = rawName.slice(0, idx);
|
||||
export function parseToolName(
|
||||
rawName: string,
|
||||
knownServerNames?: readonly string[],
|
||||
): ParsedToolName {
|
||||
if (rawName.includes(Constants.mcp_delimiter)) {
|
||||
const [toolName, mcpServer = ''] = splitToolCallName(rawName, knownServerNames);
|
||||
return { raw: rawName, mcpServer, toolName };
|
||||
}
|
||||
const friendlyKey = TOOL_FRIENDLY_NAME_KEYS[rawName];
|
||||
|
|
@ -74,8 +75,9 @@ export function parseToolName(rawName: string): ParsedToolName {
|
|||
export function getToolDisplayLabel(
|
||||
rawName: string,
|
||||
localize: (key: TranslationKeys) => string,
|
||||
knownServerNames?: readonly string[],
|
||||
): string {
|
||||
const parsed = parseToolName(rawName);
|
||||
const parsed = parseToolName(rawName, knownServerNames);
|
||||
if (parsed.mcpServer) return parsed.mcpServer;
|
||||
if (parsed.friendlyKey) return localize(parsed.friendlyKey);
|
||||
return parsed.toolName;
|
||||
|
|
|
|||
|
|
@ -30,7 +30,8 @@ export function extractMCPServers(agent: AgentWithTools): string[] {
|
|||
if (agent?.tools?.length) {
|
||||
for (const tool of agent.tools) {
|
||||
if (tool instanceof DynamicStructuredTool && tool.name.includes(Constants.mcp_delimiter)) {
|
||||
const serverName = tool.name.split(Constants.mcp_delimiter).pop();
|
||||
const carried = (tool as { mcpRawServerName?: string }).mcpRawServerName;
|
||||
const serverName = carried ?? tool.name.split(Constants.mcp_delimiter).pop();
|
||||
if (serverName) {
|
||||
mcpServers.add(serverName);
|
||||
}
|
||||
|
|
@ -42,7 +43,7 @@ export function extractMCPServers(agent: AgentWithTools): string[] {
|
|||
if (agent?.toolDefinitions?.length) {
|
||||
for (const toolDef of agent.toolDefinitions) {
|
||||
if (toolDef.name?.includes(Constants.mcp_delimiter)) {
|
||||
const serverName = toolDef.name.split(Constants.mcp_delimiter).pop();
|
||||
const serverName = toolDef.serverName ?? toolDef.name.split(Constants.mcp_delimiter).pop();
|
||||
if (serverName) {
|
||||
mcpServers.add(serverName);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -50,7 +50,7 @@ import {
|
|||
registerFileAuthoringTools,
|
||||
isFileAuthoringToolDefinition,
|
||||
} from './tools';
|
||||
import { normalizeServerName, requiresEphemeralUserConnection } from '~/mcp/utils';
|
||||
import { normalizeServerName, requiresEphemeralUserConnection, splitMCPToolKey } from '~/mcp/utils';
|
||||
import { registerMemoryTools, memoryToolUsageGuard } from './memory';
|
||||
import { applyBackgroundToolCalls } from './background';
|
||||
import { filterFilesByEndpointConfig } from '~/files';
|
||||
|
|
@ -1187,6 +1187,10 @@ export async function initializeAgent(
|
|||
ephemeralServerNames.add(normalizeServerName(serverName));
|
||||
}
|
||||
}
|
||||
/** Resolve the boundary against every configured server, not just the
|
||||
* ephemeral subset: a non-ephemeral name ending in an ephemeral one would
|
||||
* otherwise be misread as ephemeral. */
|
||||
const allServerNames = Object.keys(req.config?.mcpConfig ?? {}).map(normalizeServerName);
|
||||
const backgroundResult = applyBackgroundToolCalls({
|
||||
toolDefinitions,
|
||||
toolRegistry,
|
||||
|
|
@ -1197,13 +1201,8 @@ export async function initializeAgent(
|
|||
* Unknown servers stay eligible — the executor's per-instance tag is
|
||||
* the fail-safe for those. */
|
||||
excludeTool: (toolName) => {
|
||||
const delimiterIndex = toolName.indexOf(Constants.mcp_delimiter);
|
||||
if (delimiterIndex < 0) {
|
||||
return false;
|
||||
}
|
||||
return ephemeralServerNames.has(
|
||||
toolName.slice(delimiterIndex + Constants.mcp_delimiter.length),
|
||||
);
|
||||
const [, serverName] = splitMCPToolKey(toolName, allServerNames);
|
||||
return serverName != null && ephemeralServerNames.has(serverName);
|
||||
},
|
||||
});
|
||||
toolDefinitions = backgroundResult.toolDefinitions;
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ export * from './mcp/tools';
|
|||
export * from './mcp/request';
|
||||
/* Utilities */
|
||||
export * from './mcp/utils';
|
||||
export * from './mcp/context';
|
||||
export * from './utils';
|
||||
export { default as Tokenizer, countTokens } from './utils/tokenizer';
|
||||
export type { EncodingName } from './utils/tokenizer';
|
||||
|
|
|
|||
|
|
@ -67,6 +67,40 @@ describe('getUserMCPAuthMap', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('tool-key boundary', () => {
|
||||
it('resolves the plugin key from the last delimiter for a gateway-prefixed tool name', async () => {
|
||||
/** The raw upstream name carries the delimiter, so first-occurrence extraction
|
||||
* asked for `mcp_server_version_mcp_gitlab` and silently resolved no
|
||||
* customUserVars, leaving API-key/header placeholders unfilled. */
|
||||
mockGetPluginAuthMap.mockResolvedValue({});
|
||||
|
||||
await getUserMCPAuthMap({
|
||||
userId: 'user123',
|
||||
tools: ['gitlab-get_mcp_server_version_mcp_gitlab'],
|
||||
findPluginAuthsByKeys: mockFindPluginAuthsByKeys,
|
||||
});
|
||||
|
||||
expect(mockGetPluginAuthMap).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ pluginKeys: ['mcp_gitlab'] }),
|
||||
);
|
||||
});
|
||||
|
||||
it('resolves a configured server whose own name contains the delimiter', async () => {
|
||||
mockGetPluginAuthMap.mockResolvedValue({});
|
||||
|
||||
await getUserMCPAuthMap({
|
||||
userId: 'user123',
|
||||
tools: ['search_mcp_Google_mcp_Workspace'],
|
||||
serverNames: ['Google_mcp_Workspace'],
|
||||
findPluginAuthsByKeys: mockFindPluginAuthsByKeys,
|
||||
});
|
||||
|
||||
expect(mockGetPluginAuthMap).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ pluginKeys: ['mcp_Google_mcp_Workspace'] }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Edge Cases', () => {
|
||||
it('should return empty object when no tools have mcpRawServerName', async () => {
|
||||
const toolInstances = [
|
||||
|
|
|
|||
27
packages/api/src/mcp/__tests__/context.test.ts
Normal file
27
packages/api/src/mcp/__tests__/context.test.ts
Normal file
|
|
@ -0,0 +1,27 @@
|
|||
import { resolveMCPServerContext } from '../context';
|
||||
|
||||
describe('resolveMCPServerContext', () => {
|
||||
it('returns every configured name, not just the lazily-initialized ones', async () => {
|
||||
/** `ensureConfigServers` skips unmodified YAML servers, so its keys are not the
|
||||
* configured set; boundary resolution needs the full list or it goes inert. */
|
||||
const ensureConfigServers = jest.fn().mockResolvedValue({ lazyInit: { name: 'lazyInit' } });
|
||||
|
||||
const result = await resolveMCPServerContext({
|
||||
mcpConfig: { lazyInit: {}, unchangedYaml: {} } as never,
|
||||
ensureConfigServers,
|
||||
});
|
||||
|
||||
expect(result.configServers).toEqual({ lazyInit: { name: 'lazyInit' } });
|
||||
expect(result.serverNames.sort()).toEqual(['lazyInit', 'unchangedYaml']);
|
||||
expect(ensureConfigServers).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('normalizes names into the form tool keys embed', async () => {
|
||||
const result = await resolveMCPServerContext({
|
||||
mcpConfig: { 'Google MCP Workspace': {} } as never,
|
||||
ensureConfigServers: jest.fn().mockResolvedValue({}),
|
||||
});
|
||||
|
||||
expect(result.serverNames).toEqual(['Google_MCP_Workspace']);
|
||||
});
|
||||
});
|
||||
|
|
@ -2,6 +2,7 @@ import type { ParsedServerConfig } from '~/mcp/types';
|
|||
import {
|
||||
buildOAuthToolCallName,
|
||||
normalizeServerName,
|
||||
splitMCPToolKey,
|
||||
redactAllServerSecrets,
|
||||
redactServerSecrets,
|
||||
requiresUserScopedConnection,
|
||||
|
|
@ -45,6 +46,35 @@ describe('normalizeServerName', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('splitMCPToolKey', () => {
|
||||
it('should return the tool name unchanged with an undefined server name when there is no delimiter', () => {
|
||||
expect(splitMCPToolKey('plainToolName')).toEqual(['plainToolName', undefined]);
|
||||
});
|
||||
|
||||
it('should split a normal single-occurrence key the same way String.split would', () => {
|
||||
expect(splitMCPToolKey('search_mcp_myserver')).toEqual(['search', 'myserver']);
|
||||
});
|
||||
|
||||
it('should resolve a raw tool name that itself contains the delimiter substring by using the last occurrence', () => {
|
||||
// Regression test: a tool whose own (possibly gateway-prefixed) name
|
||||
// already contains "_mcp_" - e.g. LiteLLM's MCP gateway prefixes
|
||||
// aggregated tool names with "{server}-", so GitLab's own
|
||||
// "get_mcp_server_version" tool becomes "gitlab-get_mcp_server_version"
|
||||
// before LibreChat appends its own "_mcp_gitlab" suffix. A naive
|
||||
// `.split(delimiter)` produces 3 segments here and silently drops the
|
||||
// 3rd, yielding a bogus server name ("server_version" instead of
|
||||
// "gitlab"). See https://github.com/danny-avila/LibreChat/issues/14440
|
||||
expect(splitMCPToolKey('gitlab-get_mcp_server_version_mcp_gitlab')).toEqual([
|
||||
'gitlab-get_mcp_server_version',
|
||||
'gitlab',
|
||||
]);
|
||||
});
|
||||
|
||||
it('should handle a raw tool name with multiple delimiter occurrences by always taking the last segment as the server name', () => {
|
||||
expect(splitMCPToolKey('a_mcp_b_mcp_c_mcp_server')).toEqual(['a_mcp_b_mcp_c', 'server']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('buildOAuthToolCallName', () => {
|
||||
it('should prefix a simple server name with oauth_mcp_', () => {
|
||||
expect(buildOAuthToolCallName('my-server')).toBe('oauth_mcp_my-server');
|
||||
|
|
|
|||
|
|
@ -3,18 +3,22 @@ import { Constants } from 'librechat-data-provider';
|
|||
import type { PluginAuthMethods } from '@librechat/data-schemas';
|
||||
import type { GenericTool } from '@librechat/agents';
|
||||
import { getPluginAuthMap } from '~/agents/auth';
|
||||
import { splitMCPToolKey } from './utils';
|
||||
|
||||
export async function getUserMCPAuthMap({
|
||||
userId,
|
||||
tools,
|
||||
servers,
|
||||
toolInstances,
|
||||
serverNames,
|
||||
findPluginAuthsByKeys,
|
||||
}: {
|
||||
userId: string;
|
||||
tools?: (string | undefined)[];
|
||||
servers?: (string | undefined)[];
|
||||
toolInstances?: (GenericTool | null)[];
|
||||
/** Configured server names, used to resolve the tool-key boundary exactly */
|
||||
serverNames?: readonly string[];
|
||||
findPluginAuthsByKeys: PluginAuthMethods['findPluginAuthsByKeys'];
|
||||
}): Promise<Record<string, Record<string, string>>> {
|
||||
let allMcpCustomUserVars: Record<string, Record<string, string>> = {};
|
||||
|
|
@ -34,9 +38,7 @@ export async function getUserMCPAuthMap({
|
|||
if (!toolName) {
|
||||
continue;
|
||||
}
|
||||
const delimiterIndex = toolName.indexOf(Constants.mcp_delimiter);
|
||||
if (delimiterIndex === -1) continue;
|
||||
const mcpServer = toolName.slice(delimiterIndex + Constants.mcp_delimiter.length);
|
||||
const [, mcpServer] = splitMCPToolKey(toolName, serverNames);
|
||||
if (!mcpServer) continue;
|
||||
uniqueMcpServers.add(`${Constants.mcp_prefix}${mcpServer}`);
|
||||
}
|
||||
|
|
|
|||
35
packages/api/src/mcp/context.ts
Normal file
35
packages/api/src/mcp/context.ts
Normal file
|
|
@ -0,0 +1,35 @@
|
|||
import { normalizeServerName } from 'librechat-data-provider';
|
||||
import type { MCPOptions } from 'librechat-data-provider';
|
||||
import type { ParsedServerConfig } from '~/mcp/types';
|
||||
|
||||
export interface MCPServerContext {
|
||||
/** Config-source servers that needed lazy initialization. */
|
||||
configServers: Record<string, ParsedServerConfig>;
|
||||
/** Every configured server, in the normalized form tool keys are built from. */
|
||||
serverNames: string[];
|
||||
}
|
||||
|
||||
export interface ResolveMCPServerContextParams {
|
||||
mcpConfig: Record<string, MCPOptions>;
|
||||
ensureConfigServers: (
|
||||
mcpConfig: Record<string, MCPOptions>,
|
||||
) => Promise<Record<string, ParsedServerConfig>>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves the MCP server context for one request from a single config snapshot.
|
||||
*
|
||||
* `ensureConfigServers` deliberately skips unmodified YAML servers, so its keys are
|
||||
* not the configured set. Tool-key boundary resolution needs every configured name,
|
||||
* normalized the way keys embed it, or a server absent from the lazy-init result
|
||||
* silently falls back to positional parsing.
|
||||
*/
|
||||
export async function resolveMCPServerContext({
|
||||
mcpConfig,
|
||||
ensureConfigServers,
|
||||
}: ResolveMCPServerContextParams): Promise<MCPServerContext> {
|
||||
return {
|
||||
configServers: await ensureConfigServers(mcpConfig),
|
||||
serverNames: Object.keys(mcpConfig).map(normalizeServerName),
|
||||
};
|
||||
}
|
||||
|
|
@ -1,7 +1,7 @@
|
|||
import { Constants, Time } from 'librechat-data-provider';
|
||||
import { GraphEvents, StepTypes } from '@librechat/agents';
|
||||
import type * as t from '~/types';
|
||||
import { buildOAuthToolCallName } from '~/mcp/utils';
|
||||
import { buildOAuthToolCallName, splitMCPToolKey } from '~/mcp/utils';
|
||||
|
||||
export type OAuthPromptOptions = {
|
||||
expiresAt?: number;
|
||||
|
|
@ -24,7 +24,10 @@ export function getOAuthPromptExpiresAt(
|
|||
: now + Time.TWO_MINUTES;
|
||||
}
|
||||
|
||||
export function getMCPServerNamesFromTools(tools?: unknown[] | null): Set<string> {
|
||||
export function getMCPServerNamesFromTools(
|
||||
tools?: unknown[] | null,
|
||||
knownServerNames?: readonly string[],
|
||||
): Set<string> {
|
||||
const serverNames = new Set<string>();
|
||||
|
||||
for (const tool of tools ?? []) {
|
||||
|
|
@ -32,12 +35,12 @@ export function getMCPServerNamesFromTools(tools?: unknown[] | null): Set<string
|
|||
continue;
|
||||
}
|
||||
|
||||
const delimiterIndex = tool.indexOf(Constants.mcp_delimiter);
|
||||
if (delimiterIndex === -1) {
|
||||
const [, serverName] = splitMCPToolKey(tool, knownServerNames);
|
||||
if (!serverName) {
|
||||
continue;
|
||||
}
|
||||
|
||||
serverNames.add(tool.slice(delimiterIndex + Constants.mcp_delimiter.length));
|
||||
serverNames.add(serverName);
|
||||
}
|
||||
|
||||
return serverNames;
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { Constants } from 'librechat-data-provider';
|
||||
import { Constants, normalizeServerName } from 'librechat-data-provider';
|
||||
import type { ParsedServerConfig } from '~/mcp/types';
|
||||
import type { RequestBody } from '~/types';
|
||||
|
||||
|
|
@ -330,37 +330,6 @@ export function redactAllServerSecrets(
|
|||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalizes a server name to match the pattern ^[a-zA-Z0-9_.-]+$
|
||||
* This is required for Azure OpenAI models with Tool Calling
|
||||
*/
|
||||
export function normalizeServerName(serverName: string): string {
|
||||
// Check if the server name already matches the pattern
|
||||
if (/^[a-zA-Z0-9_.-]+$/.test(serverName)) {
|
||||
return serverName;
|
||||
}
|
||||
|
||||
/** Replace non-matching characters with underscores.
|
||||
This preserves the general structure while ensuring compatibility.
|
||||
Trims leading/trailing underscores
|
||||
*/
|
||||
const normalized = serverName.replace(/[^a-zA-Z0-9_.-]/g, '_').replace(/^_+|_+$/g, '');
|
||||
|
||||
// If the result is empty (e.g., all characters were non-ASCII and got trimmed),
|
||||
// generate a fallback name to ensure we always have a valid function name
|
||||
if (!normalized) {
|
||||
/** Hash of the original name to ensure uniqueness */
|
||||
let hash = 0;
|
||||
for (let i = 0; i < serverName.length; i++) {
|
||||
hash = (hash << 5) - hash + serverName.charCodeAt(i);
|
||||
hash |= 0; // Convert to 32bit integer
|
||||
}
|
||||
return `server_${Math.abs(hash)}`;
|
||||
}
|
||||
|
||||
return normalized;
|
||||
}
|
||||
|
||||
/**
|
||||
* Builds the synthetic tool-call name used during MCP OAuth flows.
|
||||
* Format: `oauth<mcp_delimiter><normalizedServerName>`
|
||||
|
|
@ -439,3 +408,5 @@ export function generateServerNameFromTitle(title: string): string {
|
|||
|
||||
return slug || 'mcp-server'; // Fallback if empty
|
||||
}
|
||||
|
||||
export { splitMCPToolKey, normalizeServerName } from 'librechat-data-provider';
|
||||
|
|
|
|||
|
|
@ -101,6 +101,8 @@ interface MCPToolInstance {
|
|||
mcp?: boolean;
|
||||
/** Original JSON schema attached at MCP tool creation time */
|
||||
mcpJsonSchema?: JsonSchemaType;
|
||||
/** Server this tool came from, carried from resolution instead of re-parsed */
|
||||
mcpRawServerName?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -121,7 +123,7 @@ export function extractMCPToolDefinition(tool: MCPToolInstance): ToolDefinition
|
|||
def.parameters = tool.mcpJsonSchema;
|
||||
}
|
||||
|
||||
const serverName = getServerNameFromTool(tool.name);
|
||||
const serverName = tool.mcpRawServerName ?? getServerNameFromTool(tool.name);
|
||||
if (serverName) {
|
||||
def.serverName = serverName;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@
|
|||
*/
|
||||
|
||||
import { Providers } from '@librechat/agents';
|
||||
import { Constants, isActionTool } from 'librechat-data-provider';
|
||||
import { Constants, isActionTool, splitMCPToolKey } from 'librechat-data-provider';
|
||||
import type { LCToolRegistry, JsonSchemaType, LCTool, GenericTool } from '@librechat/agents';
|
||||
import type { AgentToolOptions } from 'librechat-data-provider';
|
||||
import type { ToolDefinition } from './classification';
|
||||
|
|
@ -42,6 +42,8 @@ export interface LoadToolDefinitionsParams {
|
|||
codeExecutionEnabled?: boolean;
|
||||
/** Agent provider — Gemini/Vertex tool schemas get union-flattened for compatibility */
|
||||
provider?: Providers;
|
||||
/** Configured server names, used to resolve the tool-key boundary exactly */
|
||||
mcpServerNames?: readonly string[];
|
||||
}
|
||||
|
||||
export interface ActionToolDefinition {
|
||||
|
|
@ -87,6 +89,7 @@ export async function loadToolDefinitions(
|
|||
programmaticToolsEnabled = false,
|
||||
codeExecutionEnabled = false,
|
||||
provider,
|
||||
mcpServerNames,
|
||||
} = params;
|
||||
const { getOrFetchMCPServerTools, isBuiltInTool, getActionToolDefinitions } = deps;
|
||||
|
||||
|
|
@ -155,8 +158,8 @@ export async function loadToolDefinitions(
|
|||
continue;
|
||||
}
|
||||
|
||||
const parts = toolName.split(Constants.mcp_delimiter);
|
||||
const serverName = parts[parts.length - 1];
|
||||
const [, parsedServerName] = splitMCPToolKey(toolName, mcpServerNames);
|
||||
const serverName = parsedServerName ?? toolName;
|
||||
|
||||
if (!mcpServerToolsCache.has(serverName)) {
|
||||
const serverTools = await getOrFetchMCPServerTools(userId, serverName);
|
||||
|
|
@ -207,6 +210,7 @@ export async function loadToolDefinitions(
|
|||
description: def.description,
|
||||
mcp: true as const,
|
||||
mcpJsonSchema: def.parameters,
|
||||
mcpRawServerName: def.serverName,
|
||||
})) as unknown as GenericTool[];
|
||||
|
||||
const classificationResult = await buildToolClassification({
|
||||
|
|
|
|||
|
|
@ -2755,6 +2755,107 @@ export enum Constants {
|
|||
CHECK_BACKGROUND_TASK = 'check_background_task',
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalizes a server name into the character set tool keys are built from.
|
||||
* Tool keys embed this output, so any candidate list matched against a key must
|
||||
* be normalized the same way.
|
||||
*/
|
||||
export function normalizeServerName(serverName: string): string {
|
||||
if (/^[a-zA-Z0-9_.-]+$/.test(serverName)) {
|
||||
return serverName;
|
||||
}
|
||||
|
||||
const normalized = serverName.replace(/[^a-zA-Z0-9_.-]/g, '_').replace(/^_+|_+$/g, '');
|
||||
if (normalized) {
|
||||
return normalized;
|
||||
}
|
||||
|
||||
/** All characters were stripped; hash the original so the name stays unique. */
|
||||
let hash = 0;
|
||||
for (let i = 0; i < serverName.length; i++) {
|
||||
hash = (hash << 5) - hash + serverName.charCodeAt(i);
|
||||
hash |= 0;
|
||||
}
|
||||
return `server_${Math.abs(hash)}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Splits a combined MCP tool key (`${rawToolName}${mcp_delimiter}${serverName}`)
|
||||
* back into its two parts.
|
||||
*
|
||||
* Both halves can legitimately contain the delimiter, so position alone cannot
|
||||
* identify the boundary. Raw tool names come from the upstream server and are
|
||||
* untrusted (`get_mcp_server_version`, or a gateway-prefixed
|
||||
* `gitlab-get_mcp_server_version`), and `normalizeServerName` preserves
|
||||
* underscores, so a configured server may be named `Google_mcp_Workspace`.
|
||||
*
|
||||
* When `knownServerNames` is supplied the boundary is resolved against it: the
|
||||
* longest configured name the key actually ends with wins. Otherwise this falls
|
||||
* back to the last delimiter, which is correct whenever only the tool half
|
||||
* contains one and matches `.split()` when neither does.
|
||||
*
|
||||
* One case stays undecidable from the key alone: if both `bar` and `foo_mcp_bar`
|
||||
* are configured, `tool_mcp_foo_mcp_bar` is a valid key for either. Longest match
|
||||
* is the deterministic tiebreak; resolving it properly needs the tool/server
|
||||
* mapping carried alongside the key rather than re-derived from the string.
|
||||
*/
|
||||
export function splitMCPToolKey(
|
||||
toolKey: string,
|
||||
knownServerNames?: readonly string[],
|
||||
): [string, string | undefined] {
|
||||
if (knownServerNames?.length) {
|
||||
let matched: string | undefined;
|
||||
for (let i = 0; i < knownServerNames.length; i++) {
|
||||
const serverName = knownServerNames[i];
|
||||
if (!serverName || serverName.length <= (matched?.length ?? 0)) {
|
||||
continue;
|
||||
}
|
||||
if (toolKey.endsWith(`${Constants.mcp_delimiter}${serverName}`)) {
|
||||
matched = serverName;
|
||||
}
|
||||
}
|
||||
if (matched != null) {
|
||||
return [
|
||||
toolKey.slice(0, toolKey.length - matched.length - Constants.mcp_delimiter.length),
|
||||
matched,
|
||||
];
|
||||
}
|
||||
}
|
||||
|
||||
const idx = toolKey.lastIndexOf(Constants.mcp_delimiter);
|
||||
if (idx === -1) {
|
||||
return [toolKey, undefined];
|
||||
}
|
||||
return [toolKey.slice(0, idx), toolKey.slice(idx + Constants.mcp_delimiter.length)];
|
||||
}
|
||||
|
||||
/**
|
||||
* Splits a tool-call name for display, where the key may be a synthetic MCP OAuth
|
||||
* call (`oauth${mcp_delimiter}${serverName}`) rather than a real tool key.
|
||||
*
|
||||
* A configured server name is authoritative when one matches, because a real tool key
|
||||
* always ends in its server. Only when none matches does the `oauth` prefix decide,
|
||||
* which keeps a genuine upstream tool named `oauth${mcp_delimiter}...` from being read
|
||||
* as a synthetic call while still resolving OAuth prompts for unconfigured servers.
|
||||
*/
|
||||
export function splitToolCallName(
|
||||
toolCallName: string,
|
||||
knownServerNames?: readonly string[],
|
||||
): [string, string | undefined] {
|
||||
if (knownServerNames?.length) {
|
||||
const [toolName, serverName] = splitMCPToolKey(toolCallName, knownServerNames);
|
||||
if (serverName != null && knownServerNames.includes(serverName)) {
|
||||
return [toolName, serverName];
|
||||
}
|
||||
}
|
||||
|
||||
const oauthPrefix = `oauth${Constants.mcp_delimiter}`;
|
||||
if (toolCallName.startsWith(oauthPrefix)) {
|
||||
return ['oauth', toolCallName.slice(oauthPrefix.length)];
|
||||
}
|
||||
return splitMCPToolKey(toolCallName, knownServerNames);
|
||||
}
|
||||
|
||||
/** Maximum explicit subagent hops allowed from any root agent at runtime. */
|
||||
export const MAX_SUBAGENT_DEPTH = 5;
|
||||
|
||||
|
|
|
|||
137
packages/data-provider/src/splitMCPToolKey.spec.ts
Normal file
137
packages/data-provider/src/splitMCPToolKey.spec.ts
Normal file
|
|
@ -0,0 +1,137 @@
|
|||
import { Constants, splitMCPToolKey, splitToolCallName } from './config';
|
||||
|
||||
describe('splitMCPToolKey', () => {
|
||||
it('splits a normal single-delimiter key like String.split would', () => {
|
||||
expect(splitMCPToolKey('search_mcp_myserver')).toEqual(['search', 'myserver']);
|
||||
});
|
||||
|
||||
it('returns an undefined server name when there is no delimiter', () => {
|
||||
expect(splitMCPToolKey('plainToolName')).toEqual(['plainToolName', undefined]);
|
||||
});
|
||||
|
||||
it('resolves a raw tool name containing the delimiter via the last occurrence', () => {
|
||||
expect(splitMCPToolKey('gitlab-get_mcp_server_version_mcp_gitlab')).toEqual([
|
||||
'gitlab-get_mcp_server_version',
|
||||
'gitlab',
|
||||
]);
|
||||
});
|
||||
|
||||
it('resolves a server name containing the delimiter when known names are supplied', () => {
|
||||
expect(splitMCPToolKey('search_mcp_Google_mcp_Workspace', ['Google_mcp_Workspace'])).toEqual([
|
||||
'search',
|
||||
'Google_mcp_Workspace',
|
||||
]);
|
||||
});
|
||||
|
||||
it('prefers the longest matching configured server name', () => {
|
||||
expect(
|
||||
splitMCPToolKey('search_mcp_Google_mcp_Workspace', ['Workspace', 'Google_mcp_Workspace']),
|
||||
).toEqual(['search', 'Google_mcp_Workspace']);
|
||||
});
|
||||
|
||||
it('falls back to the last delimiter when no configured name matches', () => {
|
||||
expect(splitMCPToolKey('gitlab-get_mcp_server_version_mcp_gitlab', ['other'])).toEqual([
|
||||
'gitlab-get_mcp_server_version',
|
||||
'gitlab',
|
||||
]);
|
||||
});
|
||||
|
||||
it('still resolves the tool half when both halves contain the delimiter', () => {
|
||||
expect(splitMCPToolKey('a_mcp_b_mcp_Google_mcp_Workspace', ['Google_mcp_Workspace'])).toEqual([
|
||||
'a_mcp_b',
|
||||
'Google_mcp_Workspace',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('splitToolCallName', () => {
|
||||
const d = Constants.mcp_delimiter;
|
||||
|
||||
it('treats a synthetic OAuth call as oauth plus the full server name', () => {
|
||||
expect(splitToolCallName(`oauth${d}foo${d}bar`)).toEqual(['oauth', `foo${d}bar`]);
|
||||
});
|
||||
|
||||
it('keeps a normalized server name that itself contains the delimiter', () => {
|
||||
expect(splitToolCallName(`oauth${d}oauth${d}server`)).toEqual(['oauth', `oauth${d}server`]);
|
||||
});
|
||||
|
||||
it('resolves a real tool key whose raw name contains the delimiter', () => {
|
||||
expect(splitToolCallName(`gitlab-get${d}server_version${d}gitlab`)).toEqual([
|
||||
`gitlab-get${d}server_version`,
|
||||
'gitlab',
|
||||
]);
|
||||
});
|
||||
|
||||
it('resolves a real tool key against configured server names when supplied', () => {
|
||||
expect(splitToolCallName(`search${d}Google${d}Workspace`, [`Google${d}Workspace`])).toEqual([
|
||||
'search',
|
||||
`Google${d}Workspace`,
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('splitToolCallName with configured server names', () => {
|
||||
const d = Constants.mcp_delimiter;
|
||||
|
||||
it('reads a real tool whose own name starts with the oauth prefix', () => {
|
||||
expect(splitToolCallName(`oauth${d}reset${d}github`, ['github'])).toEqual([
|
||||
`oauth${d}reset`,
|
||||
'github',
|
||||
]);
|
||||
});
|
||||
|
||||
it('still resolves a synthetic OAuth call for a configured server', () => {
|
||||
expect(splitToolCallName(`oauth${d}github`, ['github'])).toEqual(['oauth', 'github']);
|
||||
});
|
||||
|
||||
it('resolves a synthetic OAuth call for a delimiter-bearing configured server', () => {
|
||||
expect(splitToolCallName(`oauth${d}foo${d}bar`, [`foo${d}bar`])).toEqual([
|
||||
'oauth',
|
||||
`foo${d}bar`,
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('splitMCPToolKey boundary alignment', () => {
|
||||
const d = Constants.mcp_delimiter;
|
||||
|
||||
it('ignores a configured name that is not delimiter-aligned in the key', () => {
|
||||
/** `server` is a bare suffix of `myserver`, not a segment. Matching on
|
||||
* `endsWith(name)` instead of `endsWith(delimiter + name)` would route the
|
||||
* call to a different configured server than the agent authorized. */
|
||||
expect(splitMCPToolKey(`search${d}myserver`, ['server'])).toEqual(['search', 'myserver']);
|
||||
});
|
||||
|
||||
it('treats an empty known-name list the same as no list', () => {
|
||||
expect(splitMCPToolKey(`gitlab-get${d}server_version${d}gitlab`, [])).toEqual([
|
||||
`gitlab-get${d}server_version`,
|
||||
'gitlab',
|
||||
]);
|
||||
});
|
||||
|
||||
it('requires the configured name to be normalized to match the key', () => {
|
||||
/** Keys embed `normalizeServerName`'s output, so callers must normalize their
|
||||
* candidate list; a raw name with spaces can never align. */
|
||||
expect(splitMCPToolKey(`search${d}Google_mcp_Workspace`, ['Google_mcp_Workspace'])).toEqual([
|
||||
'search',
|
||||
'Google_mcp_Workspace',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('splitToolCallName oauth precedence', () => {
|
||||
const d = Constants.mcp_delimiter;
|
||||
|
||||
it('falls back to the oauth prefix when a list is supplied but nothing matches', () => {
|
||||
/** Pins the precedence rule: the configured branch must not return its
|
||||
* last-delimiter result when no configured name actually matched. */
|
||||
expect(splitToolCallName(`oauth${d}foo${d}bar`, ['github'])).toEqual(['oauth', `foo${d}bar`]);
|
||||
});
|
||||
|
||||
it('prefers a matching configured server over the oauth prefix', () => {
|
||||
expect(splitToolCallName(`oauth${d}reset${d}github`, ['github'])).toEqual([
|
||||
`oauth${d}reset`,
|
||||
'github',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
|
@ -549,6 +549,65 @@ describe('Agent Methods', () => {
|
|||
expect(newAgent.mcpServerNames).toEqual(['authorizedServer']);
|
||||
});
|
||||
|
||||
test('should derive the server from a key whose raw tool name contains the delimiter', async () => {
|
||||
const { agentId, authorId } = createTestIds();
|
||||
/** DB server names are slugs and cannot contain the delimiter, so the trailing
|
||||
* segment is the real server even when the raw tool name carries one. Shared-agent
|
||||
* access is keyed off this field, so it must not be dropped. */
|
||||
const gatewayTool = `get${Constants.mcp_delimiter}server_version${Constants.mcp_delimiter}gitlab`;
|
||||
|
||||
const newAgent = await createAgent({
|
||||
id: agentId,
|
||||
name: 'Gateway MCP Agent',
|
||||
provider: 'test',
|
||||
model: 'test-model',
|
||||
author: authorId,
|
||||
tools: [gatewayTool],
|
||||
});
|
||||
|
||||
expect(newAgent.mcpServerNames).toEqual(['gitlab']);
|
||||
});
|
||||
|
||||
test('should preserve a resolved server name across an update that omits it', async () => {
|
||||
const { agentId, authorId } = createTestIds();
|
||||
/** Any caller that writes `tools` without `mcpServerNames` — the Action edit
|
||||
* path, for one — must not have a configured `Google_mcp_Workspace` reduced to
|
||||
* `Workspace`, which ServerConfigsDB would resolve as an unrelated DB server. */
|
||||
const mcpTool = `search${Constants.mcp_delimiter}Google${Constants.mcp_delimiter}Workspace`;
|
||||
await createAgent({
|
||||
id: agentId,
|
||||
name: 'Provenance Agent',
|
||||
provider: 'test',
|
||||
model: 'test-model',
|
||||
author: authorId,
|
||||
tools: [mcpTool],
|
||||
mcpServerNames: [`Google${Constants.mcp_delimiter}Workspace`],
|
||||
});
|
||||
|
||||
const updated = await updateAgent({ id: agentId }, { tools: [mcpTool, 'web_search'] });
|
||||
|
||||
expect(updated!.mcpServerNames).toEqual([`Google${Constants.mcp_delimiter}Workspace`]);
|
||||
expect(updated!.mcpServerNames).not.toContain('Workspace');
|
||||
});
|
||||
|
||||
test('should drop a resolved name once its last tool is gone', async () => {
|
||||
const { agentId, authorId } = createTestIds();
|
||||
const mcpTool = `search${Constants.mcp_delimiter}Google${Constants.mcp_delimiter}Workspace`;
|
||||
await createAgent({
|
||||
id: agentId,
|
||||
name: 'Provenance Agent 2',
|
||||
provider: 'test',
|
||||
model: 'test-model',
|
||||
author: authorId,
|
||||
tools: [mcpTool],
|
||||
mcpServerNames: [`Google${Constants.mcp_delimiter}Workspace`],
|
||||
});
|
||||
|
||||
const updated = await updateAgent({ id: agentId }, { tools: ['web_search'] });
|
||||
|
||||
expect(updated!.mcpServerNames).toEqual([]);
|
||||
});
|
||||
|
||||
test('should derive mcpServerNames only from MCP tools on update', async () => {
|
||||
const { agentId, authorId } = createTestIds();
|
||||
const actionTool = `sync${Constants.mcp_delimiter}state${actionDelimiter}api---example---com`;
|
||||
|
|
|
|||
|
|
@ -137,6 +137,12 @@ function extractMCPServerNames(tools: string[] | undefined | null): string[] {
|
|||
continue;
|
||||
}
|
||||
const parts = tool.split(mcp_delimiter);
|
||||
/** This index only grants DB-backed servers (`ServerConfigsDB.getAccessibleServers`),
|
||||
* and DB server names are slugs that cannot contain the delimiter
|
||||
* (`generateServerNameFromTitle` strips underscores), so the last segment is always
|
||||
* the real server for those. A config server whose own name contains the delimiter
|
||||
* yields a trailing segment that is not its name; resolving that needs the configured
|
||||
* server list, which is unavailable here - see #14449. */
|
||||
if (parts.length >= 2) {
|
||||
serverNames.add(parts[parts.length - 1]);
|
||||
}
|
||||
|
|
@ -144,6 +150,43 @@ function extractMCPServerNames(tools: string[] | undefined | null): string[] {
|
|||
return Array.from(serverNames);
|
||||
}
|
||||
|
||||
/**
|
||||
* Rebuilds an agent's MCP server index across a tools update without re-deriving
|
||||
* names from the keys.
|
||||
*
|
||||
* A name already on the agent was resolved against the registry when it was
|
||||
* stored, so it is authoritative; it carries forward while some retained tool
|
||||
* still resolves to it. Only keys that match none of them fall back to the
|
||||
* ambiguous trailing-segment derivation, which cannot tell a config server's
|
||||
* suffix from a real DB server name.
|
||||
*/
|
||||
function rebuildMCPServerNames(tools: string[] | undefined | null, priorNames: string[]): string[] {
|
||||
if (priorNames.length === 0) {
|
||||
return extractMCPServerNames(tools);
|
||||
}
|
||||
|
||||
const retained = new Set<string>();
|
||||
const unmatched: string[] = [];
|
||||
for (const tool of tools ?? []) {
|
||||
if (!tool || !tool.includes(mcp_delimiter) || isActionTool(tool)) {
|
||||
continue;
|
||||
}
|
||||
const match = priorNames
|
||||
.filter((name) => tool.endsWith(`${mcp_delimiter}${name}`))
|
||||
.sort((a, b) => b.length - a.length)[0];
|
||||
if (match) {
|
||||
retained.add(match);
|
||||
} else {
|
||||
unmatched.push(tool);
|
||||
}
|
||||
}
|
||||
|
||||
for (const name of extractMCPServerNames(unmatched)) {
|
||||
retained.add(name);
|
||||
}
|
||||
return Array.from(retained);
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a version already exists in the versions array, excluding timestamp and author fields.
|
||||
*/
|
||||
|
|
@ -440,7 +483,11 @@ export function createAgentMethods(
|
|||
},
|
||||
],
|
||||
category: (agentData.category as string) || 'general',
|
||||
mcpServerNames: extractMCPServerNames(agentData.tools as string[] | undefined),
|
||||
/** Callers that authorized the tools pass resolved names; deriving from the key
|
||||
* alone cannot tell a config server's suffix from a real DB server name. */
|
||||
mcpServerNames:
|
||||
(agentData.mcpServerNames as string[] | undefined) ??
|
||||
extractMCPServerNames(agentData.tools as string[] | undefined),
|
||||
};
|
||||
|
||||
return (await Agent.create(initialAgentData)).toObject() as IAgent;
|
||||
|
|
@ -595,9 +642,17 @@ export function createAgentMethods(
|
|||
|
||||
// Sync mcpServerNames when tools are updated
|
||||
if ((directUpdates as Record<string, unknown>).tools !== undefined) {
|
||||
const mcpServerNames = extractMCPServerNames(
|
||||
(directUpdates as Record<string, unknown>).tools as string[],
|
||||
);
|
||||
/** Callers that authorized the tools pass resolved names; deriving from the key
|
||||
* alone cannot tell a config server's suffix from a real DB server name. */
|
||||
const supplied = (directUpdates as Record<string, unknown>).mcpServerNames as
|
||||
| string[]
|
||||
| undefined;
|
||||
const mcpServerNames =
|
||||
supplied ??
|
||||
rebuildMCPServerNames(
|
||||
(directUpdates as Record<string, unknown>).tools as string[],
|
||||
(currentAgent.mcpServerNames as string[] | undefined) ?? [],
|
||||
);
|
||||
(directUpdates as Record<string, unknown>).mcpServerNames = mcpServerNames;
|
||||
updateData.mcpServerNames = mcpServerNames;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue