fix(mcp-chat): loop tool calls until a final answer

workspaces/mcp-chat/.changeset/fix-multi-round-tool-loop.md

@@ -0,0 +1,5 @@
+---
+'@backstage-community/plugin-mcp-chat-backend': patch
+---
+
+Fix multi-step tool calls dropping the second tool call and leaking raw tool-call tokens; processQuery now loops with tools until the model returns a final answer. The loop's iteration cap is configurable via `mcpChat.maxToolIterations` (default 8).

workspaces/mcp-chat/app-config.yaml

@@ -27,6 +27,7 @@ integrations:
 # --------- MCP Chat Configuration ---------
 mcpChat:
   toolCallTimeout: 60000
+  maxToolIterations: 8
   providers:
     - id: openai
       baseUrl: 'https://any-openai-compatible-url/v1'

workspaces/mcp-chat/docs/SERVER_CONFIGURATION.md

@@ -177,6 +177,15 @@ mcpChat:
   toolCallTimeout: 300000 # 5 minutes, in milliseconds
 ```
 
+### Max Tool Iterations
+
+When answering a question, the assistant runs an agentic loop that executes tool calls and feeds the results back to the model until it produces a final answer. This loop is capped at **8 iterations** by default to prevent runaway tool-calling. For questions that legitimately require many sequential tool calls, raise the cap with `maxToolIterations`:
+
+```yaml
+mcpChat:
+  maxToolIterations: 16
+```
+
 ---
 
 For additional setup instructions and troubleshooting, refer to the [main README](../README.md).

workspaces/mcp-chat/plugins/mcp-chat-backend/config.d.ts

@@ -134,6 +134,14 @@ export interface Config {
      * @default 60000
      */
     toolCallTimeout?: number;
+    /**
+     * Maximum number of tool-call iterations the agentic loop will run before
+     * giving up on producing a final answer. Increase this for questions that
+     * legitimately require many sequential tool calls.
+     * @visibility backend
+     * @default 8
+     */
+    maxToolIterations?: number;
     /**
      * Custom system prompt for the AI assistant
      * @visibility backend

workspaces/mcp-chat/plugins/mcp-chat-backend/report.api.md

@@ -122,6 +122,9 @@ export interface ConversationRecord {
 // @public
 export function createRouter(options: RouterOptions): Promise<express.Router>;
 
+// @public
+export const DEFAULT_MCP_MAX_TOOL_ITERATIONS = 8;
+
 // @public
 export const DEFAULT_MCP_TOOL_CALL_TIMEOUT_MS = 60000;
 

workspaces/mcp-chat/plugins/mcp-chat-backend/src/index.ts

@@ -110,6 +110,7 @@ export {
   executeToolCall,
   findNpxPath,
   DEFAULT_MCP_TOOL_CALL_TIMEOUT_MS,
+  DEFAULT_MCP_MAX_TOOL_ITERATIONS,
 } from './utils';
 
 // =============================================================================

workspaces/mcp-chat/plugins/mcp-chat-backend/src/services/MCPClientServiceImpl.test.ts

@@ -253,6 +253,58 @@ describe('MCPClientServiceImpl', () => {
       expect(result.toolResponses[0].serverId).toBe('error');
     });
 
+    it('should not throw when a failing tool call has malformed JSON arguments', async () => {
+      const toolCall: ToolCall = {
+        id: 'call_bad_json',
+        type: 'function',
+        function: {
+          name: 'failing_tool',
+          arguments: 'not valid json',
+        },
+      };
+
+      const initialResponse: ChatResponse = {
+        choices: [
+          {
+            message: {
+              role: 'assistant',
+              content: null,
+              tool_calls: [toolCall],
+            },
+          },
+        ],
+      };
+
+      const followUpResponse: ChatResponse = {
+        choices: [
+          {
+            message: {
+              role: 'assistant',
+              content: 'Recovered after the tool error.',
+            },
+          },
+        ],
+      };
+
+      mockLLMProvider.sendMessage
+        .mockResolvedValueOnce(initialResponse)
+        .mockResolvedValueOnce(followUpResponse);
+
+      utils.executeToolCall.mockRejectedValue(
+        new Error('Tool execution failed'),
+      );
+
+      const result = await service.processQuery([
+        { role: 'user', content: 'Use the failing tool' },
+      ]);
+
+      expect(result.reply).toBe('Recovered after the tool error.');
+      expect(result.toolResponses[0].serverId).toBe('error');
+      expect(result.toolResponses[0].arguments).toEqual({
+        _raw: 'not valid json',
+      });
+    });
+
     it('should handle LLM provider errors', async () => {
       mockLLMProvider.sendMessage.mockRejectedValue(
         new Error('LLM connection failed'),
@@ -262,6 +314,215 @@ describe('MCPClientServiceImpl', () => {
         service.processQuery([{ role: 'user', content: 'Hello' }]),
       ).rejects.toThrow('LLM connection failed');
     });
+
+    it('should pass tools on every sendMessage call (multi-round loop)', async () => {
+      const toolCall1: ToolCall = {
+        id: 'call_1',
+        type: 'function',
+        function: { name: 'tool_one', arguments: '{"a": 1}' },
+      };
+      const toolCall2: ToolCall = {
+        id: 'call_2',
+        type: 'function',
+        function: { name: 'tool_two', arguments: '{"b": 2}' },
+      };
+
+      const firstResponse: ChatResponse = {
+        choices: [
+          {
+            message: {
+              role: 'assistant',
+              content: null,
+              tool_calls: [toolCall1],
+            },
+          },
+        ],
+      };
+      const secondResponse: ChatResponse = {
+        choices: [
+          {
+            message: {
+              role: 'assistant',
+              content: null,
+              tool_calls: [toolCall2],
+            },
+          },
+        ],
+      };
+      const finalResponse: ChatResponse = {
+        choices: [
+          {
+            message: {
+              role: 'assistant',
+              content: 'Both tools executed successfully.',
+            },
+          },
+        ],
+      };
+
+      mockLLMProvider.sendMessage
+        .mockResolvedValueOnce(firstResponse)
+        .mockResolvedValueOnce(secondResponse)
+        .mockResolvedValueOnce(finalResponse);
+
+      const result = await service.processQuery([
+        { role: 'user', content: 'Use both tools' },
+      ]);
+
+      expect(result.reply).toBe('Both tools executed successfully.');
+      expect(result.toolCalls).toHaveLength(2);
+      expect(result.toolResponses).toHaveLength(2);
+
+      // All three sendMessage calls must receive tools (the key fix)
+      expect(mockLLMProvider.sendMessage).toHaveBeenCalledTimes(3);
+      for (const call of mockLLMProvider.sendMessage.mock.calls) {
+        expect(call[1]).toBeDefined();
+        expect(Array.isArray(call[1])).toBe(true);
+      }
+
+      expect(utils.executeToolCall).toHaveBeenCalledTimes(2);
+    });
+
+    it('should record one assistant message with all tool calls followed by tool messages', async () => {
+      const toolCall1: ToolCall = {
+        id: 'call_a',
+        type: 'function',
+        function: { name: 'tool_a', arguments: '{"x": 1}' },
+      };
+      const toolCall2: ToolCall = {
+        id: 'call_b',
+        type: 'function',
+        function: { name: 'tool_b', arguments: '{"y": 2}' },
+      };
+
+      const firstResponse: ChatResponse = {
+        choices: [
+          {
+            message: {
+              role: 'assistant',
+              content: null,
+              tool_calls: [toolCall1, toolCall2],
+            },
+          },
+        ],
+      };
+      const finalResponse: ChatResponse = {
+        choices: [
+          {
+            message: {
+              role: 'assistant',
+              content: 'Done.',
+            },
+          },
+        ],
+      };
+
+      mockLLMProvider.sendMessage
+        .mockResolvedValueOnce(firstResponse)
+        .mockResolvedValueOnce(finalResponse);
+
+      utils.executeToolCall.mockImplementation((toolCall: ToolCall) =>
+        Promise.resolve({
+          id: toolCall.id,
+          name: toolCall.function.name,
+          arguments: {},
+          result: `result for ${toolCall.id}`,
+          serverId: 'test-server',
+        }),
+      );
+
+      await service.processQuery([{ role: 'user', content: 'Use both tools' }]);
+
+      // Inspect the conversation handed to the second (final) LLM round.
+      const secondRoundMessages = mockLLMProvider.sendMessage.mock.calls[1][0];
+      const appended = secondRoundMessages.slice(-3);
+
+      expect(appended[0]).toEqual({
+        role: 'assistant',
+        content: null,
+        tool_calls: [toolCall1, toolCall2],
+      });
+      expect(appended[1]).toEqual({
+        role: 'tool',
+        content: 'result for call_a',
+        tool_call_id: 'call_a',
+      });
+      expect(appended[2]).toEqual({
+        role: 'tool',
+        content: 'result for call_b',
+        tool_call_id: 'call_b',
+      });
+    });
+
+    it('should return fallback message when max tool iterations reached', async () => {
+      const toolCall: ToolCall = {
+        id: 'call_loop',
+        type: 'function',
+        function: { name: 'infinite_tool', arguments: '{}' },
+      };
+      const toolResponse: ChatResponse = {
+        choices: [
+          {
+            message: {
+              role: 'assistant',
+              content: null,
+              tool_calls: [toolCall],
+            },
+          },
+        ],
+      };
+
+      // Always return a tool call so the loop never terminates naturally
+      mockLLMProvider.sendMessage.mockResolvedValue(toolResponse);
+
+      const result = await service.processQuery([
+        { role: 'user', content: 'Loop forever' },
+      ]);
+
+      expect(result.reply).toContain(
+        "couldn't compile a final answer within the allowed number of steps",
+      );
+      expect(mockLLMProvider.sendMessage).toHaveBeenCalledTimes(8);
+    });
+
+    it('should honor a configured maxToolIterations cap', async () => {
+      mockConfig.getOptionalNumber.mockImplementation((key: string) =>
+        key === 'mcpChat.maxToolIterations' ? 3 : undefined,
+      );
+      service = new MCPClientServiceImpl({
+        logger: mockLogger,
+        config: mockConfig,
+      });
+
+      const toolCall: ToolCall = {
+        id: 'call_loop',
+        type: 'function',
+        function: { name: 'infinite_tool', arguments: '{}' },
+      };
+      const toolResponse: ChatResponse = {
+        choices: [
+          {
+            message: {
+              role: 'assistant',
+              content: null,
+              tool_calls: [toolCall],
+            },
+          },
+        ],
+      };
+
+      // Always return a tool call so the loop never terminates naturally
+      mockLLMProvider.sendMessage.mockResolvedValue(toolResponse);
+
+      const result = await service.processQuery([
+        { role: 'user', content: 'Loop forever' },
+      ]);
+
+      expect(result.reply).toContain(
+        "couldn't compile a final answer within the allowed number of steps",
+      );
+      expect(mockLLMProvider.sendMessage).toHaveBeenCalledTimes(3);
+    });
   });
 
   describe('Status Reporting', () => {