diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index c2173095fb..f818cc3833 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -71,6 +71,43 @@ const { getLogStores } = require('~/cache'); const domainSeparatorRegex = new RegExp(actionDomainSeparator, 'g'); +/** + * Collapse every `actionDomainSeparator` sequence in a fully-qualified + * action tool name to an underscore. Agents can store tool names in the + * raw `domainParser(..., true)` output, which for short hostnames is a + * `---`-separated string (e.g. `medium---com`). The lookup maps below + * are always keyed with the `_`-collapsed form, so every read must + * normalize first or short-hostname tools silently fail to resolve. + */ +const normalizeActionToolName = (toolName) => toolName.replace(domainSeparatorRegex, '_'); + +/** + * Populate a `toolToAction` map with one slot per fully-qualified tool + * name (``). Both the new + * and the legacy encodings of the domain are registered for every + * function so agents whose stored tool names predate the current + * encoding still resolve correctly. + * + * Indexing on the full tool name instead of the encoded domain alone is + * what makes multi-action agents work when two actions share a hostname: + * the operationId disambiguates them, so neither overwrites the other. + */ +const registerActionTools = ({ + toolToAction, + functionSignatures, + normalizedDomain, + legacyNormalized, + makeEntry, +}) => { + for (const sig of functionSignatures) { + const entry = makeEntry(sig); + toolToAction.set(`${sig.name}${actionDelimiter}${normalizedDomain}`, entry); + if (legacyNormalized !== normalizedDomain) { + toolToAction.set(`${sig.name}${actionDelimiter}${legacyNormalized}`, entry); + } + } +}; + /** * Resolves the set of enabled agent capabilities from endpoints config, * falling back to app-level or default capabilities for ephemeral agents. @@ -328,37 +365,32 @@ async function processRequiredActions(client, requiredActions) { const decryptedAction = { ...action }; decryptedAction.metadata = await decryptMetadata(action.metadata); - for (const sig of functionSignatures) { - const entry = { + registerActionTools({ + toolToAction, + functionSignatures, + normalizedDomain, + legacyNormalized, + makeEntry: (sig) => ({ action: decryptedAction, requestBuilder: requestBuilders[sig.name], encrypted, - }; - toolToAction.set(`${sig.name}${actionDelimiter}${normalizedDomain}`, entry); - if (legacyNormalized !== normalizedDomain) { - toolToAction.set(`${sig.name}${actionDelimiter}${legacyNormalized}`, entry); - } - } + }), + }); // Store builders for reuse ActionBuildersMap[action.metadata.domain] = requestBuilders; } - actionSetsData = { toolToAction }; + actionSetsData = toolToAction; } - const entry = actionSetsData.toolToAction.get(currentAction.tool); + const entry = actionSetsData.get(normalizeActionToolName(currentAction.tool)); if (!entry) { continue; } const { action, requestBuilder, encrypted } = entry; - if (!requestBuilder) { - // throw new Error(`Tool ${currentAction.tool} not found.`); - continue; - } - // We've already decrypted the metadata, so we can pass it directly const _allowedDomains = appConfig?.actions?.allowedDomains; tool = await createActionTool({ @@ -1058,19 +1090,19 @@ async function loadAgentTools({ true, ); - for (const sig of functionSignatures) { - const entry = { + registerActionTools({ + toolToAction, + functionSignatures, + normalizedDomain, + legacyNormalized, + makeEntry: (sig) => ({ action: decryptedAction, requestBuilder: requestBuilders[sig.name], zodSchema: zodSchemas[sig.name], functionSignature: sig, encrypted, - }; - toolToAction.set(`${sig.name}${actionDelimiter}${normalizedDomain}`, entry); - if (legacyNormalized !== normalizedDomain) { - toolToAction.set(`${sig.name}${actionDelimiter}${legacyNormalized}`, entry); - } - } + }), + }); } // Now map tools to the processed action sets @@ -1081,7 +1113,7 @@ async function loadAgentTools({ continue; } - const entry = toolToAction.get(toolName); + const entry = toolToAction.get(normalizeActionToolName(toolName)); if (!entry) { continue; } @@ -1382,23 +1414,23 @@ async function loadActionToolsForExecution({ true, ); - for (const sig of functionSignatures) { - const entry = { + registerActionTools({ + toolToAction, + functionSignatures, + normalizedDomain, + legacyNormalized, + makeEntry: (sig) => ({ action: decryptedAction, requestBuilder: requestBuilders[sig.name], zodSchema: zodSchemas[sig.name], functionSignature: sig, encrypted, - }; - toolToAction.set(`${sig.name}${actionDelimiter}${normalizedDomain}`, entry); - if (legacyNormalized !== normalizedDomain) { - toolToAction.set(`${sig.name}${actionDelimiter}${legacyNormalized}`, entry); - } - } + }), + }); } for (const toolName of actionToolNames) { - const entry = toolToAction.get(toolName); + const entry = toolToAction.get(normalizeActionToolName(toolName)); if (!entry) { continue; } @@ -1413,7 +1445,7 @@ async function loadActionToolsForExecution({ encrypted, requestBuilder, name: toolName, - description: functionSignature?.description ?? '', + description: functionSignature.description, useSSRFProtection: !Array.isArray(allowedDomains) || allowedDomains.length === 0, }); diff --git a/api/server/services/__tests__/ToolService.spec.js b/api/server/services/__tests__/ToolService.spec.js index f38b900bff..730fe88cf5 100644 --- a/api/server/services/__tests__/ToolService.spec.js +++ b/api/server/services/__tests__/ToolService.spec.js @@ -80,6 +80,7 @@ jest.mock('~/cache', () => ({ const { loadAgentTools, loadToolsForExecution, + processRequiredActions, resolveAgentCapabilities, } = require('../ToolService'); @@ -687,5 +688,115 @@ describe('ToolService - Action Capability Gating', () => { expect(mockCreateActionTool).toHaveBeenCalledTimes(2); expectBothActionsResolved(mockCreateActionTool.mock.calls); }); + + it('processRequiredActions resolves both actions when they share a hostname', async () => { + // The assistants/threads path received the same structural rewrite + // as the agent paths. Cover it directly so future regressions in the + // `toolToAction` map shape or the lookup normalization don't slip + // through just because the agent-path tests still pass. + mockLoadActionSets.mockResolvedValue([actionA, actionB]); + const client = { + req: { + user: { id: 'user_123' }, + body: { + assistant_id: 'assistant_collision', + model: 'gpt-4o-mini', + endpoint: 'openAI', + }, + config: {}, + }, + res: {}, + apiKey: 'sk-test', + mappedOrder: new Map(), + seenToolCalls: new Map(), + addContentData: jest.fn(), + }; + + await processRequiredActions(client, [ + { + tool: toolNameA, + toolInput: {}, + toolCallId: 'call_a', + thread_id: 'thread_1', + run_id: 'run_1', + }, + { + tool: toolNameB, + toolInput: {}, + toolCallId: 'call_b', + thread_id: 'thread_1', + run_id: 'run_1', + }, + ]); + + // The assistants path intentionally doesn't forward `name` to + // createActionTool (see ToolService.js — "intentionally not passing + // zodSchema, name, and description for assistants API"), so key + // resolution assertions off the request builder path instead. + expect(mockCreateActionTool).toHaveBeenCalledTimes(2); + const builderPaths = mockCreateActionTool.mock.calls.map( + (c) => c[0].requestBuilder?.path, + ); + expect(builderPaths).toEqual(expect.arrayContaining(['/echo', '/items'])); + // Each call must carry a distinct builder — guards against the bug + // where the surviving action's builders got routed to every tool. + expect(builderPaths[0]).not.toBe(builderPaths[1]); + }); + + it('loadAgentTools resolves legacy-format tool names via the legacy encoding branch', async () => { + // Agents whose tool names predate the current domain encoding store + // them under `legacyDomainEncode`'s output. The map registers both + // encodings per function so these keep resolving after the fix; + // this test exercises the `if (legacyNormalized !== normalizedDomain)` + // branch, which was previously never hit by any test. + mockLoadActionSets.mockResolvedValue([actionA]); + const legacyToolName = `echoMessage${actionDelimiter}${LEGACY_ENCODED_DOMAIN}`; + const capabilities = [AgentCapabilities.tools, AgentCapabilities.actions]; + const req = createMockReq(capabilities); + mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities)); + + await loadAgentTools({ + req, + res: {}, + agent: { id: 'agent_legacy', tools: [legacyToolName] }, + definitionsOnly: false, + }); + + expect(mockCreateActionTool).toHaveBeenCalledTimes(1); + const [callArgs] = mockCreateActionTool.mock.calls[0]; + expect(callArgs.name).toBe(legacyToolName); + expect(callArgs.requestBuilder.path).toBe('/echo'); + }); + + it('loadAgentTools resolves raw `---`-separated tool names from agent.tools', async () => { + // Hostnames at or below ENCODED_DOMAIN_LENGTH round-trip through + // `domainParser(..., true)` as a `---`-separated string, and agents + // persist that raw form in `agent.tools`. The map is always keyed + // with the `_`-collapsed form, so the lookup must normalize the + // incoming name or short-hostname tools silently drop out. + mockDomainParser.mockResolvedValue('shared---dom'); + mockLoadActionSets.mockResolvedValue([actionA, actionB]); + const rawNameA = `echoMessage${actionDelimiter}shared---dom`; + const rawNameB = `listItems${actionDelimiter}shared---dom`; + const capabilities = [AgentCapabilities.tools, AgentCapabilities.actions]; + const req = createMockReq(capabilities); + mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities)); + + await loadAgentTools({ + req, + res: {}, + agent: { id: 'agent_short', tools: [rawNameA, rawNameB] }, + definitionsOnly: false, + }); + + expect(mockCreateActionTool).toHaveBeenCalledTimes(2); + const callsByName = new Map( + mockCreateActionTool.mock.calls.map((c) => [c[0].name, c[0]]), + ); + expect(callsByName.has(rawNameA)).toBe(true); + expect(callsByName.has(rawNameB)).toBe(true); + expect(callsByName.get(rawNameA).requestBuilder.path).toBe('/echo'); + expect(callsByName.get(rawNameB).requestBuilder.path).toBe('/items'); + }); }); });