Fix ACP/Zed: task tool crash + todo_pause loop-break (Closes #2653)

.gitignore

@@ -103,3 +103,6 @@ evals/logs/
 
 # Git worktrees (ad-hoc local checkouts; keep out of git and test discovery)
 .worktrees/
+
+# ACP test logs (from scripts/test-acp-zed-bugs.mjs)
+.acp-test-logs/

packages/agents/src/core/agenticLoop/AgenticLoop.ts

@@ -51,6 +51,7 @@ import {
   buildToolResponses,
   classifyCompletedTools,
   recordCancelledToolHistory,
+  recordCompletedToolHistory,
 } from './loopHelpers.js';
 
 const logger = new DebugLogger('llxprt:agents:agentic-loop');
@@ -639,6 +640,17 @@ export class AgenticLoop {
       await recordCancelledToolHistory(agentTools, this.agentClient);
       return { continueLoop: false, nextMessage: [] };
     }
+    // A successful pause request (the pause tool) is a terminal signal: the
+    // loop must stop so the agent returns control to the user. Without this,
+    // the loop feeds the pause response back to the model and continues
+    // indefinitely (issue #2653). In the CLI this is masked by the React UI
+    // continuation gate, but ACP/Zed and other headless consumers have none.
+    if (hasSuccessfulTodoPause(agentTools)) {
+      // Eagerly persist tool history since there will be no next model turn
+      // to carry the response parts naturally.
+      await recordCompletedToolHistory(agentTools, this.agentClient);
+      return { continueLoop: false, nextMessage: [] };
+    }
     const responseParts = buildToolResponses(agentTools);
     if (responseParts.length === 0) {
       return { continueLoop: false, nextMessage: [] };
@@ -655,3 +667,18 @@ function* flushBuffered(queue: EventQueue): Generator<AgenticLoopEvent> {
     event = queue.popBuffered();
   }
 }
+
+/**
+ * Returns true if any of the completed tool calls is a successful pause
+ * (case-insensitive, no error). A failed pause must NOT terminate the loop,
+ * matching TodoContinuationService.isSuccessfulTodoPauseResponse semantics.
+ */
+function hasSuccessfulTodoPause(tools: CompletedToolCall[]): boolean {
+  return tools.some(
+    (tc) =>
+      tc.request.name.toLowerCase() === 'todo_pause' &&
+      tc.status === 'success' &&
+      tc.response.error === undefined &&
+      tc.response.errorType === undefined,
+  );
+}

packages/agents/src/core/agenticLoop/__tests__/agenticLoop.todoPause.test.ts

@@ -0,0 +1,243 @@
+/**
+ * @license
+ * Copyright 2025 Vybestack LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/**
+ * Behavioral tests for AgenticLoop pause-loop-break (issue #2653).
+ *
+ * These tests verify that when the model calls the pause tool and the tool
+ * completes successfully, the AgenticLoop stops — it does NOT feed the
+ * pause response back into another model turn. This is critical for
+ * ACP/Zed and other headless consumers that lack the CLI's React UI
+ * continuation gate.
+ *
+ * The test approach follows dev-docs/RULES.md: real AgenticLoop, real
+ * CoreToolScheduler, real MessageBus. The only mock boundary is the
+ * provider stream (scripted ServerAgentStreamEvents).
+ */
+
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { AgenticLoop } from '../AgenticLoop.js';
+import { MockTool } from '@vybestack/llxprt-code-core/test-utils/mock-tool.js';
+import { clearAllSchedulers } from '@vybestack/llxprt-code-core/config/schedulerSingleton.js';
+import { MessageBus } from '@vybestack/llxprt-code-core/confirmation-bus/message-bus.js';
+import { ApprovalMode } from '@vybestack/llxprt-code-core/config/configTypes.js';
+import {
+  createScriptedAgentClient,
+  createTestConfig,
+  createToolRegistryForTest,
+  createAskPolicyEngine,
+  collectEvents,
+  isToolsComplete,
+  toolCallRequestEvent,
+  finishedEvent,
+} from './agenticLoop-test-helpers.js';
+
+describe('AgenticLoop pause loop-break (issue #2653)', () => {
+  beforeEach(() => {
+    clearAllSchedulers();
+  });
+  afterEach(() => {
+    clearAllSchedulers();
+  });
+
+  it('stops the loop after a successful pause tool call (no extra model turn)', async () => {
+    const pauseTool = new MockTool({
+      name: 'todo_pause',
+      execute: async () => ({
+        llmContent: 'paused',
+        returnDisplay: 'paused',
+      }),
+    });
+
+    const toolRegistry = createToolRegistryForTest([pauseTool]);
+    const messageBus = new MessageBus(createAskPolicyEngine(), false);
+    const config = createTestConfig({
+      messageBus,
+      toolRegistry,
+      policyEngine: createAskPolicyEngine(),
+      interactive: true,
+      approvalMode: ApprovalMode.YOLO,
+    });
+
+    // Turn 1: model requests the pause tool, then finishes the turn.
+    // No Turn 2 script is provided — if the loop tries to continue, the
+    // scripted client will return an empty stream.
+    const { client, turnMessages } = createScriptedAgentClient([
+      [
+        toolCallRequestEvent('todo_pause', 'pause-1', {
+          reason: 'testing pause',
+        }),
+        finishedEvent(),
+      ],
+    ]);
+
+    const loop = new AgenticLoop({
+      agentClient: client,
+      config,
+      messageBus,
+      interactiveMode: true,
+    });
+
+    const events = await collectEvents(
+      loop,
+      [{ type: 'text', text: 'pause please' }],
+      new AbortController().signal,
+    );
+
+    expect(events.some(isToolsComplete)).toBe(true);
+    // CRITICAL: only ONE model turn should have happened. If the loop
+    // continued after the pause, turnMessages would have 2 entries.
+    expect(turnMessages).toHaveLength(1);
+
+    const errorEvents = events.filter((e) => e.kind === 'error');
+    expect(errorEvents).toHaveLength(0);
+  });
+
+  it('eagerly records pause tool history so the response is durable', async () => {
+    const pauseTool = new MockTool({
+      name: 'todo_pause',
+      execute: async () => ({
+        llmContent: 'paused with reason',
+        returnDisplay: 'paused',
+      }),
+    });
+
+    const toolRegistry = createToolRegistryForTest([pauseTool]);
+    const messageBus = new MessageBus(createAskPolicyEngine(), false);
+    const config = createTestConfig({
+      messageBus,
+      toolRegistry,
+      policyEngine: createAskPolicyEngine(),
+      interactive: true,
+      approvalMode: ApprovalMode.YOLO,
+    });
+
+    const { client, history } = createScriptedAgentClient([
+      [
+        toolCallRequestEvent('todo_pause', 'pause-2', {
+          reason: 'need to stop',
+        }),
+        finishedEvent(),
+      ],
+    ]);
+
+    const loop = new AgenticLoop({
+      agentClient: client,
+      config,
+      messageBus,
+      interactiveMode: true,
+    });
+
+    await collectEvents(
+      loop,
+      [{ type: 'text', text: 'pause' }],
+      new AbortController().signal,
+    );
+
+    // History must contain the tool response blocks, even though there was
+    // no next model turn to carry them. This proves the eager history
+    // recording in buildNextMessage works.
+    const allBlocks = history.flatMap((h) => h.blocks);
+    const hasToolResponse = allBlocks.some((b) => b.type === 'tool_response');
+    expect(hasToolResponse).toBe(true);
+  });
+
+  it('does NOT stop the loop when the pause tool fails (error response)', async () => {
+    const pauseTool = new MockTool({
+      name: 'todo_pause',
+      execute: async () => {
+        throw new Error('pause failed');
+      },
+    });
+
+    const echoTool = new MockTool({
+      name: 'echo',
+      execute: async () => ({
+        llmContent: 'echoed',
+        returnDisplay: 'echoed',
+      }),
+    });
+
+    const toolRegistry = createToolRegistryForTest([pauseTool, echoTool]);
+    const messageBus = new MessageBus(createAskPolicyEngine(), false);
+    const config = createTestConfig({
+      messageBus,
+      toolRegistry,
+      policyEngine: createAskPolicyEngine(),
+      interactive: true,
+      approvalMode: ApprovalMode.YOLO,
+    });
+
+    const { client, turnMessages } = createScriptedAgentClient([
+      [
+        toolCallRequestEvent('todo_pause', 'pause-err', {
+          reason: 'will fail',
+        }),
+        finishedEvent(),
+      ],
+      [toolCallRequestEvent('echo', 'echo-1', {}), finishedEvent()],
+    ]);
+
+    const loop = new AgenticLoop({
+      agentClient: client,
+      config,
+      messageBus,
+      interactiveMode: true,
+    });
+
+    await collectEvents(
+      loop,
+      [{ type: 'text', text: 'try pause then echo' }],
+      new AbortController().signal,
+    );
+
+    // The failed pause must NOT have terminated the loop — the model should
+    // have received the error and continued with a second turn.
+    expect(turnMessages.length).toBeGreaterThanOrEqual(2);
+  });
+
+  it('continues the loop normally for non-pause tools after the fix', async () => {
+    // Regression guard: a normal successful tool should still cause the
+    // loop to continue (feed response, next model turn).
+    const normalTool = new MockTool({
+      name: 'get_info',
+      execute: async () => ({
+        llmContent: 'info result',
+        returnDisplay: 'info result',
+      }),
+    });
+
+    const toolRegistry = createToolRegistryForTest([normalTool]);
+    const messageBus = new MessageBus(createAskPolicyEngine(), false);
+    const config = createTestConfig({
+      messageBus,
+      toolRegistry,
+      policyEngine: createAskPolicyEngine(),
+      interactive: true,
+      approvalMode: ApprovalMode.YOLO,
+    });
+
+    const { client, turnMessages } = createScriptedAgentClient([
+      [toolCallRequestEvent('get_info', 'info-1', {}), finishedEvent()],
+      [finishedEvent()],
+    ]);
+
+    const loop = new AgenticLoop({
+      agentClient: client,
+      config,
+      messageBus,
+      interactiveMode: true,
+    });
+
+    await collectEvents(
+      loop,
+      [{ type: 'text', text: 'get info' }],
+      new AbortController().signal,
+    );
+
+    expect(turnMessages).toHaveLength(2);
+  });
+});

packages/agents/src/core/agenticLoop/loopHelpers.ts

@@ -96,7 +96,7 @@ export function buildToolResponses(
  * Awaits both writes so callers that continue or exit the loop immediately
  * afterwards can guarantee the tool history is durable before the next turn.
  */
-async function recordCompletedToolHistory(
+export async function recordCompletedToolHistory(
   tools: CompletedToolCall[],
   agentClient: AgentClientContract,
 ): Promise<void> {