🎯 fix: MCP Tool Misclassification from Action Delimiter Collision (#12512)

* fix: prevent MCP tools with `_action` in name from being misclassified as OpenAPI action tools

Add `isActionTool()` helper that checks for the `_action_` delimiter
while guarding against cross-delimiter collision with `_mcp_`. Replace
all `includes(actionDelimiter)` classification checks with the new
helper across backend and frontend.

* test: add coverage for MCP/action cross-delimiter collision

Verify that `isActionTool` correctly rejects MCP tool names containing
`_action` and that `loadAgentTools` does not filter them based on
`actionsEnabled`. Add ToolIcon and definitions test cases.

* fix: simplify isActionTool to handle all MCP name patterns

- Use `!toolName.includes('_mcp_')` instead of checking only after the
  first `_action_` occurrence, which missed MCP tools with `_action_` in
  the middle of their name (e.g. `get_action_data_mcp_myserver`).
- Reference `Constants.mcp_delimiter` value via a local const to avoid
  circular import from config.ts, with a comment explaining why.
- Remove dead `actionDelimiter` import from definitions.ts.
- Replace double-filter with single-pass partition in loadToolsForExecution.
- Add test for mid-name `_action_` collision case.

* fix: narrow MCP exclusion to delimiter position in isActionTool

Only reject when `_mcp_` appears after `_action_` (the MCP suffix
position). `_mcp_` before `_action_` is part of the operationId and
is valid — e.g. `sync_mcp_state_action_api---example---com` is a
legitimate action tool whose operationId happens to contain `_mcp_`.

* fix: document positional _mcp_ guard and known RFC-invalid domain limitation

Expand JSDoc on isActionTool to explain the action/MCP format
disambiguation and the theoretical false negative for non-RFC-compliant
domains containing `_mcp_`. Add test documenting this known edge case.

api/server/services/ToolService.js

@@ -31,6 +31,7 @@ const {
   imageGenTools,
   EModelEndpoint,
   EToolResources,
+  isActionTool,
   actionDelimiter,
   ImageVisionTool,
   openapiToFunction,
@@ -490,7 +491,7 @@ async function loadToolDefinitionsWrapper({ req, res, agent, streamId = null, to
     if (tool === Tools.web_search) {
       return checkCapability(AgentCapabilities.web_search);
     }
-    if (tool.includes(actionDelimiter)) {
+    if (isActionTool(tool)) {
       return actionsEnabled;
     }
     if (!areToolsEnabled) {
@@ -871,7 +872,7 @@ async function loadAgentTools({
     } else if (tool === Tools.web_search) {
       includesWebSearch = checkCapability(AgentCapabilities.web_search);
       return includesWebSearch;
-    } else if (tool.includes(actionDelimiter)) {
+    } else if (isActionTool(tool)) {
       return actionsEnabled;
     } else if (!areToolsEnabled) {
       return false;
@@ -978,7 +979,7 @@ async function loadAgentTools({
 
   agentTools.push(...additionalTools);
 
-  const hasActionTools = _agentTools.some((t) => t.includes(actionDelimiter));
+  const hasActionTools = _agentTools.some((t) => isActionTool(t));
   if (!hasActionTools) {
     return {
       toolRegistry,
@@ -1237,8 +1238,11 @@ async function loadToolsForExecution({
     ? [...new Set([...requestedNonSpecialToolNames, ...ptcOrchestratedToolNames])]
     : requestedNonSpecialToolNames;
 
-  const actionToolNames = allToolNamesToLoad.filter((name) => name.includes(actionDelimiter));
-  const regularToolNames = allToolNamesToLoad.filter((name) => !name.includes(actionDelimiter));
+  const actionToolNames = [];
+  const regularToolNames = [];
+  for (const name of allToolNamesToLoad) {
+    (isActionTool(name) ? actionToolNames : regularToolNames).push(name);
+  }
 
   if (regularToolNames.length > 0) {
     const includesWebSearch = regularToolNames.includes(Tools.web_search);

api/server/services/__tests__/ToolService.spec.js

@@ -2,6 +2,7 @@ const {
   Tools,
   Constants,
   EModelEndpoint,
+  isActionTool,
   actionDelimiter,
   AgentCapabilities,
   defaultAgentCapabilities,
@@ -143,6 +144,42 @@ describe('ToolService - Action Capability Gating', () => {
     });
   });
 
+  describe('isActionTool — cross-delimiter collision guard', () => {
+    it('should identify real action tools', () => {
+      expect(isActionTool(`get_weather${actionDelimiter}api_example_com`)).toBe(true);
+      expect(isActionTool(`fetch_data${actionDelimiter}my---domain---com`)).toBe(true);
+    });
+
+    it('should identify action tools whose operationId contains _mcp_', () => {
+      expect(isActionTool(`sync_mcp_state${actionDelimiter}api---example---com`)).toBe(true);
+      expect(isActionTool(`get_mcp_config${actionDelimiter}internal---api---com`)).toBe(true);
+    });
+
+    it('should reject MCP tools whose name ends with _action', () => {
+      expect(isActionTool(`get_action${Constants.mcp_delimiter}myserver`)).toBe(false);
+      expect(isActionTool(`fetch_action${Constants.mcp_delimiter}server_name`)).toBe(false);
+      expect(isActionTool(`retrieve_action${Constants.mcp_delimiter}srv`)).toBe(false);
+    });
+
+    it('should reject MCP tools with _action_ in the middle of their name', () => {
+      expect(isActionTool(`get_action_data${Constants.mcp_delimiter}myserver`)).toBe(false);
+      expect(isActionTool(`create_action_item${Constants.mcp_delimiter}server`)).toBe(false);
+    });
+
+    it('should reject tools without the action delimiter', () => {
+      expect(isActionTool('calculator')).toBe(false);
+      expect(isActionTool(`web_search${Constants.mcp_delimiter}myserver`)).toBe(false);
+    });
+
+    it('known limitation: non-RFC domain with _mcp_ substring yields false negative', () => {
+      // RFC 952/1123 prohibit underscores in hostnames, so this is not expected in practice.
+      // Encoded domain `api_mcp_internal_com` places `_mcp_` after `_action_`, which
+      // the guard interprets as the MCP suffix.
+      const edgeCaseTool = `getData${actionDelimiter}api_mcp_internal_com`;
+      expect(isActionTool(edgeCaseTool)).toBe(false);
+    });
+  });
+
   describe('loadAgentTools (definitionsOnly=true) — action tool filtering', () => {
     const actionToolName = `get_weather${actionDelimiter}api_example_com`;
     const regularTool = 'calculator';
@@ -183,6 +220,25 @@ describe('ToolService - Action Capability Gating', () => {
       expect(callArgs.tools).toContain(actionToolName);
     });
 
+    it('should not filter MCP tools whose name contains _action (cross-delimiter collision)', async () => {
+      const mcpToolWithAction = `get_action${Constants.mcp_delimiter}myserver`;
+      const capabilities = [AgentCapabilities.tools];
+      const req = createMockReq(capabilities);
+      mockGetEndpointsConfig.mockResolvedValue(createEndpointsConfig(capabilities));
+
+      await loadAgentTools({
+        req,
+        res: {},
+        agent: { id: 'agent_123', tools: [regularTool, mcpToolWithAction] },
+        definitionsOnly: true,
+      });
+
+      expect(mockLoadToolDefinitions).toHaveBeenCalledTimes(1);
+      const [callArgs] = mockLoadToolDefinitions.mock.calls[0];
+      expect(callArgs.tools).toContain(mcpToolWithAction);
+      expect(callArgs.tools).toContain(regularTool);
+    });
+
     it('should return actionsEnabled in the result', async () => {
       const capabilities = [AgentCapabilities.tools];
       const req = createMockReq(capabilities);