diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index b4d948eda4..7478564292 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -71,6 +71,83 @@ const { getLogStores } = require('~/cache'); const domainSeparatorRegex = new RegExp(actionDomainSeparator, 'g'); +/** + * Collapse every `actionDomainSeparator` sequence in the encoded-domain + * suffix of 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 domain, so every read must normalize that suffix or + * short-hostname tools silently fail to resolve. + * + * The operationId portion (everything before the last `actionDelimiter`) + * is deliberately left untouched: `openapiToFunction` preserves hyphens + * in generated operationIds, so two specs can legitimately produce + * operationIds that differ only in hyphens-vs-underscores (e.g. + * `get_foo---bar` vs `get_foo_bar`). Collapsing the operationId would + * merge those into a single map slot and silently drop one tool. + */ +const normalizeActionToolName = (toolName) => { + const delimiterIndex = toolName.lastIndexOf(actionDelimiter); + if (delimiterIndex === -1) { + return toolName; + } + const prefixEnd = delimiterIndex + actionDelimiter.length; + const encodedDomain = toolName.slice(prefixEnd); + return toolName.slice(0, prefixEnd) + encodedDomain.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. + * + * Two actions that additionally share the same operationId still + * collide (nothing in the key distinguishes them). That case is + * pathological — `sanitizeOperationId` plus OpenAPI's own uniqueness + * requirement make it very unlikely — but when it does happen we log + * a warning so the silent-overwrite mode from the original bug cannot + * reappear under a different disguise. + */ +const registerActionTools = ({ + toolToAction, + functionSignatures, + normalizedDomain, + legacyNormalized, + makeEntry, +}) => { + const setKey = (key, entry) => { + if (toolToAction.has(key)) { + logger.warn( + `[Actions] operationId collision: "${key}" already registered; ` + + `action "${entry.action?.action_id}" overwrites the previous entry. ` + + `Two actions share both the operationId and the encoded hostname.`, + ); + } + toolToAction.set(key, entry); + }; + + for (const sig of functionSignatures) { + const entry = makeEntry(sig); + // Use `sig.name` verbatim: `openapiToFunction` keeps hyphens in + // generated operationIds, so `get_foo---bar` and `get_foo_bar` are + // distinct operations on the same spec. `normalizeActionToolName` + // only touches the encoded-domain suffix at lookup time, so map + // keys and lookups stay consistent without merging distinct + // operationIds into the same slot. + setKey(`${sig.name}${actionDelimiter}${normalizedDomain}`, entry); + if (legacyNormalized !== normalizedDomain) { + setKey(`${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. @@ -271,19 +348,14 @@ async function processRequiredActions(client, requiredActions) { assistant_id: client.req.body.assistant_id, })) ?? []; - // Process all action sets once - // Map domains to their processed action sets - const processedDomains = new Map(); - const domainLookupMap = new Map(); + // See registerActionTools for the key-shape rationale. + const toolToAction = new Map(); for (const action of actionSets) { const domain = await domainParser(action.metadata.domain, true); - domainLookupMap.set(domain, domain); - + const normalizedDomain = domain.replace(domainSeparatorRegex, '_'); const legacyDomain = legacyDomainEncode(action.metadata.domain); - if (legacyDomain !== domain) { - domainLookupMap.set(legacyDomain, domain); - } + const legacyNormalized = legacyDomain.replace(domainSeparatorRegex, '_'); const isDomainAllowed = await isActionDomainAllowed( action.metadata.domain, @@ -316,7 +388,7 @@ async function processRequiredActions(client, requiredActions) { } // Process the OpenAPI spec - const { requestBuilders } = openapiToFunction(validationResult.spec); + const { requestBuilders, functionSignatures } = openapiToFunction(validationResult.spec); // Store encrypted values for OAuth flow const encrypted = { @@ -328,42 +400,31 @@ async function processRequiredActions(client, requiredActions) { const decryptedAction = { ...action }; decryptedAction.metadata = await decryptMetadata(action.metadata); - processedDomains.set(domain, { - action: decryptedAction, - requestBuilders, - encrypted, + registerActionTools({ + toolToAction, + functionSignatures, + normalizedDomain, + legacyNormalized, + makeEntry: (sig) => ({ + action: decryptedAction, + requestBuilder: requestBuilders[sig.name], + encrypted, + }), }); // Store builders for reuse ActionBuildersMap[action.metadata.domain] = requestBuilders; } - actionSetsData = { domainLookupMap, processedDomains }; + actionSetsData = toolToAction; } - let currentDomain = ''; - let matchedKey = ''; - for (const [key, canonical] of actionSetsData.domainLookupMap.entries()) { - if (currentAction.tool.includes(key)) { - currentDomain = canonical; - matchedKey = key; - break; - } - } - - if (!currentDomain || !actionSetsData.processedDomains.has(currentDomain)) { + const entry = actionSetsData.get(normalizeActionToolName(currentAction.tool)); + if (!entry) { continue; } - const { action, requestBuilders, encrypted } = - actionSetsData.processedDomains.get(currentDomain); - const functionName = currentAction.tool.replace(`${actionDelimiter}${matchedKey}`, ''); - const requestBuilder = requestBuilders[functionName]; - - if (!requestBuilder) { - // throw new Error(`Tool ${currentAction.tool} not found.`); - continue; - } + const { action, requestBuilder, encrypted } = entry; // We've already decrypted the metadata, so we can pass it directly const _allowedDomains = appConfig?.actions?.allowedDomains; @@ -1008,17 +1069,15 @@ async function loadAgentTools({ }; } - const processedActionSets = new Map(); - const domainLookupMap = new Map(); + // See registerActionTools for the key-shape rationale. + const toolToAction = new Map(); for (const action of actionSets) { const domain = await domainParser(action.metadata.domain, true); - domainLookupMap.set(domain, domain); - + const normalizedDomain = domain.replace(domainSeparatorRegex, '_'); const legacyDomain = legacyDomainEncode(action.metadata.domain); - if (legacyDomain !== domain) { - domainLookupMap.set(legacyDomain, domain); - } + const legacyNormalized = legacyDomain.replace(domainSeparatorRegex, '_'); + const isDomainAllowed = await isActionDomainAllowed( action.metadata.domain, appConfig?.actions?.allowedDomains, @@ -1063,12 +1122,18 @@ async function loadAgentTools({ true, ); - processedActionSets.set(domain, { - action: decryptedAction, - requestBuilders, + registerActionTools({ + toolToAction, functionSignatures, - zodSchemas, - encrypted, + normalizedDomain, + legacyNormalized, + makeEntry: (sig) => ({ + action: decryptedAction, + requestBuilder: requestBuilders[sig.name], + zodSchema: zodSchemas[sig.name], + functionSignature: sig, + encrypted, + }), }); } @@ -1080,52 +1145,35 @@ async function loadAgentTools({ continue; } - let currentDomain = ''; - let matchedKey = ''; - for (const [key, canonical] of domainLookupMap.entries()) { - if (toolName.includes(key)) { - currentDomain = canonical; - matchedKey = key; - break; - } - } - - if (!currentDomain || !processedActionSets.has(currentDomain)) { + const entry = toolToAction.get(normalizeActionToolName(toolName)); + if (!entry) { continue; } - const { action, encrypted, zodSchemas, requestBuilders, functionSignatures } = - processedActionSets.get(currentDomain); - const functionName = toolName.replace(`${actionDelimiter}${matchedKey}`, ''); - const functionSig = functionSignatures.find((sig) => sig.name === functionName); - const requestBuilder = requestBuilders[functionName]; - const zodSchema = zodSchemas[functionName]; + const { action, encrypted, zodSchema, requestBuilder, functionSignature } = entry; + const _allowedDomains = appConfig?.actions?.allowedDomains; + const tool = await createActionTool({ + userId: req.user.id, + res, + action, + requestBuilder, + zodSchema, + encrypted, + name: toolName, + description: functionSignature.description, + streamId, + useSSRFProtection: !Array.isArray(_allowedDomains) || _allowedDomains.length === 0, + }); - if (requestBuilder) { - const _allowedDomains = appConfig?.actions?.allowedDomains; - const tool = await createActionTool({ - userId: req.user.id, - res, - action, - requestBuilder, - zodSchema, - encrypted, - name: toolName, - description: functionSig.description, - streamId, - useSSRFProtection: !Array.isArray(_allowedDomains) || _allowedDomains.length === 0, - }); - - if (!tool) { - logger.warn( - `Invalid action: user: ${req.user.id} | agent_id: ${agent.id} | toolName: ${toolName}`, - ); - throw new Error(`{"type":"${ErrorTypes.INVALID_ACTION}"}`); - } - - agentTools.push(tool); - ActionToolMap[toolName] = tool; + if (!tool) { + logger.warn( + `Invalid action: user: ${req.user.id} | agent_id: ${agent.id} | toolName: ${toolName}`, + ); + throw new Error(`{"type":"${ErrorTypes.INVALID_ACTION}"}`); } + + agentTools.push(tool); + ActionToolMap[toolName] = tool; } if (_agentTools.length > 0 && agentTools.length === 0) { @@ -1333,21 +1381,15 @@ async function loadActionToolsForExecution({ return loadedActionTools; } - const processedActionSets = new Map(); - /** Maps both new and legacy normalized domains to their canonical (new) domain key */ - const normalizedToDomain = new Map(); + // See registerActionTools for the key-shape rationale. + const toolToAction = new Map(); const allowedDomains = appConfig?.actions?.allowedDomains; for (const action of actionSets) { const domain = await domainParser(action.metadata.domain, true); const normalizedDomain = domain.replace(domainSeparatorRegex, '_'); - normalizedToDomain.set(normalizedDomain, domain); - const legacyDomain = legacyDomainEncode(action.metadata.domain); const legacyNormalized = legacyDomain.replace(domainSeparatorRegex, '_'); - if (legacyNormalized !== normalizedDomain) { - normalizedToDomain.set(legacyNormalized, domain); - } const isDomainAllowed = await isActionDomainAllowed(action.metadata.domain, allowedDomains); if (!isDomainAllowed) { @@ -1390,60 +1432,28 @@ async function loadActionToolsForExecution({ true, ); - processedActionSets.set(domain, { - action: decryptedAction, - requestBuilders, + registerActionTools({ + toolToAction, functionSignatures, - zodSchemas, - encrypted, + normalizedDomain, legacyNormalized, + makeEntry: (sig) => ({ + action: decryptedAction, + requestBuilder: requestBuilders[sig.name], + zodSchema: zodSchemas[sig.name], + functionSignature: sig, + encrypted, + }), }); } for (const toolName of actionToolNames) { - let currentDomain = ''; - for (const [normalizedDomain, canonicalDomain] of normalizedToDomain.entries()) { - if (toolName.includes(normalizedDomain)) { - currentDomain = canonicalDomain; - break; - } - } - - if (!currentDomain || !processedActionSets.has(currentDomain)) { - continue; - } - - const { action, encrypted, zodSchemas, requestBuilders, functionSignatures, legacyNormalized } = - processedActionSets.get(currentDomain); - const normalizedDomain = currentDomain.replace(domainSeparatorRegex, '_'); - const functionName = toolName.replace(`${actionDelimiter}${normalizedDomain}`, ''); - const functionSig = functionSignatures.find((sig) => sig.name === functionName); - const requestBuilder = requestBuilders[functionName]; - const zodSchema = zodSchemas[functionName]; - - if (!requestBuilder) { - const legacyFnName = toolName.replace(`${actionDelimiter}${legacyNormalized}`, ''); - if (legacyFnName !== toolName && requestBuilders[legacyFnName]) { - const legacyTool = await createActionTool({ - userId: req.user.id, - res, - action, - streamId, - encrypted, - requestBuilder: requestBuilders[legacyFnName], - zodSchema: zodSchemas[legacyFnName], - name: toolName, - description: - functionSignatures.find((sig) => sig.name === legacyFnName)?.description ?? '', - useSSRFProtection: !Array.isArray(allowedDomains) || allowedDomains.length === 0, - }); - if (legacyTool) { - loadedActionTools.push(legacyTool); - } - } + const entry = toolToAction.get(normalizeActionToolName(toolName)); + if (!entry) { continue; } + const { action, encrypted, zodSchema, requestBuilder, functionSignature } = entry; const tool = await createActionTool({ userId: req.user.id, res, @@ -1453,7 +1463,7 @@ async function loadActionToolsForExecution({ encrypted, requestBuilder, name: toolName, - description: functionSig?.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 740bb06e5a..63b2287684 100644 --- a/api/server/services/__tests__/ToolService.spec.js +++ b/api/server/services/__tests__/ToolService.spec.js @@ -31,6 +31,10 @@ jest.mock('~/app/clients/tools/util', () => ({ })); const mockLoadActionSets = jest.fn(); +const mockDomainParser = jest.fn(); +const mockLegacyDomainEncode = jest.fn(); +const mockDecryptMetadata = jest.fn(); +const mockCreateActionTool = jest.fn(); jest.mock('~/server/services/Tools/credentials', () => ({ loadAuthValues: jest.fn().mockResolvedValue({}), })); @@ -52,9 +56,10 @@ jest.mock('~/server/services/Files/Code/process', () => ({ })); jest.mock('../ActionService', () => ({ loadActionSets: (...args) => mockLoadActionSets(...args), - decryptMetadata: jest.fn(), - createActionTool: jest.fn(), - domainParser: jest.fn(), + decryptMetadata: (...args) => mockDecryptMetadata(...args), + createActionTool: (...args) => mockCreateActionTool(...args), + domainParser: (...args) => mockDomainParser(...args), + legacyDomainEncode: (...args) => mockLegacyDomainEncode(...args), })); jest.mock('~/server/services/Threads', () => ({ recordUsage: jest.fn(), @@ -75,6 +80,7 @@ jest.mock('~/cache', () => ({ const { loadAgentTools, loadToolsForExecution, + processRequiredActions, resolveAgentCapabilities, } = require('../ToolService'); @@ -536,4 +542,316 @@ describe('ToolService - Action Capability Gating', () => { expect(enabledCapabilities.has(AgentCapabilities.deferred_tools)).toBe(true); }); }); + + describe('multi-action domain collision regression', () => { + // Two distinct OpenAPI Actions whose `servers[0].url` resolves to the + // same hostname must both contribute their tools to the agent. The + // previous implementation indexed processed action sets by encoded + // domain, so the second action overwrote the first in the map and one + // action's tools silently disappeared from the LLM payload. + // + // The encoded domain we use as the lookup key for the action sets is + // mocked to a fixed string for both actions to make the collision + // condition deterministic without depending on the real base64 + // truncation rules. + const SHARED_DOMAIN = 'https://api.example.com'; + const ENCODED_DOMAIN = 'shared_dom'; + const LEGACY_ENCODED_DOMAIN = 'legacy_dom'; + + const buildSpec = (operationId, path) => + JSON.stringify({ + openapi: '3.0.3', + info: { title: `Mock ${operationId}`, version: '1.0.0' }, + servers: [{ url: SHARED_DOMAIN }], + paths: { + [path]: { + get: { + operationId, + summary: `Mock ${operationId}`, + responses: { + 200: { + description: 'OK', + content: { 'application/json': { schema: { type: 'object' } } }, + }, + }, + }, + }, + }, + }); + + const actionA = { + action_id: 'action_a', + metadata: { + domain: SHARED_DOMAIN, + raw_spec: buildSpec('echoMessage', '/echo'), + }, + }; + const actionB = { + action_id: 'action_b', + metadata: { + domain: SHARED_DOMAIN, + raw_spec: buildSpec('listItems', '/items'), + }, + }; + + const toolNameA = `echoMessage${actionDelimiter}${ENCODED_DOMAIN}`; + const toolNameB = `listItems${actionDelimiter}${ENCODED_DOMAIN}`; + + beforeEach(() => { + // Both actions share a hostname → both call sites get the same encoded + // value back. This is precisely the collision shape that triggered + // the bug in production. + mockDomainParser.mockResolvedValue(ENCODED_DOMAIN); + mockLegacyDomainEncode.mockReturnValue(LEGACY_ENCODED_DOMAIN); + mockDecryptMetadata.mockImplementation(async (metadata) => metadata); + mockCreateActionTool.mockImplementation(async ({ name, requestBuilder }) => ({ + name, + // Surface the request builder identity on the returned tool so + // assertions can verify each tool was wired to the correct action's + // builder, not its sibling's. + _builder: requestBuilder, + // Resolve instead of returning undefined — processRequiredActions + // chains `.then(handleToolOutput)` directly onto this call, which + // would throw synchronously on an undefined return and mask the + // test as a simulated runtime crash. + _call: jest.fn().mockResolvedValue('{"status":"ok"}'), + schema: {}, + description: '', + })); + }); + + const expectBothActionsResolved = (calls) => { + const callsByName = new Map(calls.map((c) => [c[0].name, c[0]])); + expect(callsByName.has(toolNameA)).toBe(true); + expect(callsByName.has(toolNameB)).toBe(true); + // Each tool's request builder must come from the matching action's + // own parsed spec — not the sibling's. The previous bug would either + // route both to the same action's builders (and drop one as + // undefined) or silently skip one entirely. + const builderA = callsByName.get(toolNameA).requestBuilder; + const builderB = callsByName.get(toolNameB).requestBuilder; + expect(builderA).toBeDefined(); + expect(builderB).toBeDefined(); + expect(builderA).not.toBe(builderB); + // Each builder targets its own operation path — confirms the + // request builder lookup didn't cross-contaminate between actions. + expect(builderA.path).toBe('/echo'); + expect(builderB.path).toBe('/items'); + }; + + it('loadAgentTools resolves both actions when they share a hostname', async () => { + mockLoadActionSets.mockResolvedValue([actionA, actionB]); + const capabilities = [AgentCapabilities.tools, AgentCapabilities.actions]; + const req = createMockReq(capabilities); + mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities)); + + await loadAgentTools({ + req, + res: {}, + agent: { id: 'agent_collision', tools: [toolNameA, toolNameB] }, + definitionsOnly: false, + }); + + expect(mockCreateActionTool).toHaveBeenCalledTimes(2); + expectBothActionsResolved(mockCreateActionTool.mock.calls); + }); + + it('loadAgentTools is order-invariant for two actions sharing a hostname', async () => { + // Reverse the actionSets order — what used to flip the "winner" of + // the encoded-domain Map overwrite must now make zero observable + // difference. + mockLoadActionSets.mockResolvedValue([actionB, actionA]); + const capabilities = [AgentCapabilities.tools, AgentCapabilities.actions]; + const req = createMockReq(capabilities); + mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities)); + + await loadAgentTools({ + req, + res: {}, + agent: { id: 'agent_collision', tools: [toolNameA, toolNameB] }, + definitionsOnly: false, + }); + + expect(mockCreateActionTool).toHaveBeenCalledTimes(2); + expectBothActionsResolved(mockCreateActionTool.mock.calls); + }); + + it('loadToolsForExecution resolves both actions when they share a hostname', async () => { + mockLoadActionSets.mockResolvedValue([actionA, actionB]); + const req = createMockReq([AgentCapabilities.actions]); + req.config = {}; + + await loadToolsForExecution({ + req, + res: {}, + agent: { id: 'agent_collision' }, + toolNames: [toolNameA, toolNameB], + actionsEnabled: true, + }); + + 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 distinguishes operationIds that differ only by `---` vs `_`', async () => { + // `openapiToFunction` uses the user-supplied operationId verbatim + // and only sanitizes the synthetic `_` fallback, and + // `sanitizeOperationId` preserves `-`. So two operations whose + // operationIds differ only by `---` vs `_` (e.g. `get_foo---bar` + // and `get_foo_bar`) are legitimately distinct on the same spec — + // or, here, on two actions sharing a hostname. + // + // Normalization must only touch the encoded-domain suffix after + // `actionDelimiter`; if it also collapsed the operationId, both + // tools would write to the same map slot and resolve to the + // surviving entry's request builder. + const hyphenSpec = { + action_id: 'action_hyphen', + metadata: { + domain: SHARED_DOMAIN, + raw_spec: buildSpec('get_foo---bar', '/foo-bar'), + }, + }; + const underscoreSpec = { + action_id: 'action_underscore', + metadata: { + domain: SHARED_DOMAIN, + raw_spec: buildSpec('get_foo_bar', '/foo_bar'), + }, + }; + mockLoadActionSets.mockResolvedValue([hyphenSpec, underscoreSpec]); + + const hyphenTool = `get_foo---bar${actionDelimiter}${ENCODED_DOMAIN}`; + const underscoreTool = `get_foo_bar${actionDelimiter}${ENCODED_DOMAIN}`; + const capabilities = [AgentCapabilities.tools, AgentCapabilities.actions]; + const req = createMockReq(capabilities); + mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities)); + + await loadAgentTools({ + req, + res: {}, + agent: { id: 'agent_hyphen', tools: [hyphenTool, underscoreTool] }, + definitionsOnly: false, + }); + + expect(mockCreateActionTool).toHaveBeenCalledTimes(2); + const callsByName = new Map(mockCreateActionTool.mock.calls.map((c) => [c[0].name, c[0]])); + expect(callsByName.has(hyphenTool)).toBe(true); + expect(callsByName.has(underscoreTool)).toBe(true); + expect(callsByName.get(hyphenTool).requestBuilder.path).toBe('/foo-bar'); + expect(callsByName.get(underscoreTool).requestBuilder.path).toBe('/foo_bar'); + // Critical: the two must resolve to distinct builders. If the + // operationId half of the key is normalized, both collapse to + // the same map slot and one silently overwrites the other. + expect(callsByName.get(hyphenTool).requestBuilder).not.toBe( + callsByName.get(underscoreTool).requestBuilder, + ); + }); + + 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'); + }); + }); });