Merge pull request #1383 from QwenLM/fix/tool-result-text-mode

LaZzyMan · web-flow · commit 7a823060ac8c · 2026-01-06T17:50:58.000+08:00
fix: improve tool execution feedback in non-interactive mode
diff --git a/packages/cli/src/nonInteractiveCli.test.ts b/packages/cli/src/nonInteractiveCli.test.ts
@@ -298,7 +298,9 @@ describe('runNonInteractive', () => {
       mockConfig,
       expect.objectContaining({ name: 'testTool' }),
       expect.any(AbortSignal),
-      undefined,
+      expect.objectContaining({
+        outputUpdateHandler: expect.any(Function),
+      }),
     );
     // Verify first call has isContinuation: false
     expect(mockGeminiClient.sendMessageStream).toHaveBeenNthCalledWith(
@@ -1777,4 +1779,84 @@ describe('runNonInteractive', () => {
       { isContinuation: false },
     );
   });
+
+  it('should print tool output to console in text mode (non-Task tools)', async () => {
+    // Test that tool output is printed to stdout in text mode
+    const toolCallEvent: ServerGeminiStreamEvent = {
+      type: GeminiEventType.ToolCallRequest,
+      value: {
+        callId: 'tool-1',
+        name: 'run_in_terminal',
+        args: { command: 'npm outdated' },
+        isClientInitiated: false,
+        prompt_id: 'prompt-id-tool-output',
+      },
+    };
+
+    // Mock tool execution with outputUpdateHandler being called
+    mockCoreExecuteToolCall.mockImplementation(
+      async (_config, _request, _signal, options) => {
+        // Simulate tool calling outputUpdateHandler with output chunks
+        if (options?.outputUpdateHandler) {
+          options.outputUpdateHandler('tool-1', 'Package outdated\n');
+          options.outputUpdateHandler('tool-1', 'npm@1.0.0 -> npm@2.0.0\n');
+        }
+        return {
+          responseParts: [
+            {
+              functionResponse: {
+                id: 'tool-1',
+                name: 'run_in_terminal',
+                response: {
+                  output: 'Package outdated\nnpm@1.0.0 -> npm@2.0.0',
+                },
+              },
+            },
+          ],
+        };
+      },
+    );
+
+    const firstCallEvents: ServerGeminiStreamEvent[] = [
+      toolCallEvent,
+      {
+        type: GeminiEventType.Finished,
+        value: { reason: undefined, usageMetadata: { totalTokenCount: 5 } },
+      },
+    ];
+
+    const secondCallEvents: ServerGeminiStreamEvent[] = [
+      { type: GeminiEventType.Content, value: 'Dependencies checked' },
+      {
+        type: GeminiEventType.Finished,
+        value: { reason: undefined, usageMetadata: { totalTokenCount: 3 } },
+      },
+    ];
+
+    mockGeminiClient.sendMessageStream
+      .mockReturnValueOnce(createStreamFromEvents(firstCallEvents))
+      .mockReturnValueOnce(createStreamFromEvents(secondCallEvents));
+
+    await runNonInteractive(
+      mockConfig,
+      mockSettings,
+      'Check dependencies',
+      'prompt-id-tool-output',
+    );
+
+    // Verify that executeToolCall was called with outputUpdateHandler
+    expect(mockCoreExecuteToolCall).toHaveBeenCalledWith(
+      mockConfig,
+      expect.objectContaining({ name: 'run_in_terminal' }),
+      expect.any(AbortSignal),
+      expect.objectContaining({
+        outputUpdateHandler: expect.any(Function),
+      }),
+    );
+
+    // Verify tool output was written to stdout
+    expect(processStdoutSpy).toHaveBeenCalledWith('Package outdated\n');
+    expect(processStdoutSpy).toHaveBeenCalledWith('npm@1.0.0 -> npm@2.0.0\n');
+    expect(processStdoutSpy).toHaveBeenCalledWith('Dependencies checked');
+  });
 });
diff --git a/packages/cli/src/nonInteractiveCli.ts b/packages/cli/src/nonInteractiveCli.ts
@@ -4,7 +4,11 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import type { Config, ToolCallRequestInfo } from '@qwen-code/qwen-code-core';
+import type {
+  Config,
+  ToolCallRequestInfo,
+  ToolResultDisplay,
+} from '@qwen-code/qwen-code-core';
 import { isSlashCommand } from './ui/utils/commandUtils.js';
 import type { LoadedSettings } from './config/settings.js';
 import {
@@ -333,7 +337,7 @@ export async function runNonInteractive(
                 ? options.controlService.permission.getToolCallUpdateCallback()
                 : undefined;
 
-            // Only pass outputUpdateHandler for Task tool
+            // Create output handler for Task tool (for subagent execution)
             const isTaskTool = finalRequestInfo.name === 'task';
             const taskToolProgress = isTaskTool
               ? createTaskToolProgressHandler(
@@ -343,20 +347,41 @@ export async function runNonInteractive(
                 )
               : undefined;
             const taskToolProgressHandler = taskToolProgress?.handler;
+
+            // Create output handler for non-Task tools in text mode (for console output)
+            const nonTaskOutputHandler =
+              !isTaskTool && !adapter
+                ? (callId: string, outputChunk: ToolResultDisplay) => {
+                    // Print tool output to console in text mode
+                    if (typeof outputChunk === 'string') {
+                      process.stdout.write(outputChunk);
+                    } else if (
+                      outputChunk &&
+                      typeof outputChunk === 'object' &&
+                      'ansiOutput' in outputChunk
+                    ) {
+                      // Handle ANSI output - just print as string for now
+                      process.stdout.write(String(outputChunk.ansiOutput));
+                    }
+                  }
+                : undefined;
+
+            // Combine output handlers
+            const outputUpdateHandler =
+              taskToolProgressHandler || nonTaskOutputHandler;
+
             const toolResponse = await executeToolCall(
               config,
               finalRequestInfo,
               abortController.signal,
-              isTaskTool && taskToolProgressHandler
+              outputUpdateHandler || toolCallUpdateCallback
                 ? {
-                    outputUpdateHandler: taskToolProgressHandler,
-                    onToolCallsUpdate: toolCallUpdateCallback,
-                  }
-                : toolCallUpdateCallback
-                  ? {
+                    ...(outputUpdateHandler && { outputUpdateHandler }),
+                    ...(toolCallUpdateCallback && {
                       onToolCallsUpdate: toolCallUpdateCallback,
-                    }
-                  : undefined,
+                    }),
+                  }
+                : undefined,
             );
 
             // Note: In JSON mode, subagent messages are automatically added to the main
diff --git a/packages/cli/src/utils/errors.test.ts b/packages/cli/src/utils/errors.test.ts
@@ -6,7 +6,11 @@
 
 import { vi, type Mock, type MockInstance } from 'vitest';
 import type { Config } from '@qwen-code/qwen-code-core';
-import { OutputFormat, FatalInputError } from '@qwen-code/qwen-code-core';
+import {
+  OutputFormat,
+  FatalInputError,
+  ToolErrorType,
+} from '@qwen-code/qwen-code-core';
 import {
   getErrorMessage,
   handleError,
@@ -65,6 +69,7 @@ vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => {
 describe('errors', () => {
   let mockConfig: Config;
   let processExitSpy: MockInstance;
+  let processStderrWriteSpy: MockInstance;
   let consoleErrorSpy: MockInstance;
 
   beforeEach(() => {
@@ -74,6 +79,11 @@ describe('errors', () => {
     // Mock console.error
     consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
 
+    // Mock process.stderr.write
+    processStderrWriteSpy = vi
+      .spyOn(process.stderr, 'write')
+      .mockImplementation(() => true);
+
     // Mock process.exit to throw instead of actually exiting
     processExitSpy = vi.spyOn(process, 'exit').mockImplementation((code) => {
       throw new Error(`process.exit called with code: ${code}`);
@@ -84,11 +94,13 @@ describe('errors', () => {
       getOutputFormat: vi.fn().mockReturnValue(OutputFormat.TEXT),
       getContentGeneratorConfig: vi.fn().mockReturnValue({ authType: 'test' }),
       getDebugMode: vi.fn().mockReturnValue(true),
+      isInteractive: vi.fn().mockReturnValue(false),
     } as unknown as Config;
   });
 
   afterEach(() => {
     consoleErrorSpy.mockRestore();
+    processStderrWriteSpy.mockRestore();
     processExitSpy.mockRestore();
   });
 
@@ -432,6 +444,87 @@ describe('errors', () => {
         expect(processExitSpy).not.toHaveBeenCalled();
       });
     });
+
+    describe('permission denied warnings', () => {
+      it('should show warning when EXECUTION_DENIED in non-interactive text mode', () => {
+        (mockConfig.getDebugMode as Mock).mockReturnValue(false);
+        (mockConfig.isInteractive as Mock).mockReturnValue(false);
+        (
+          mockConfig.getOutputFormat as ReturnType<typeof vi.fn>
+        ).mockReturnValue(OutputFormat.TEXT);
+
+        handleToolError(
+          toolName,
+          toolError,
+          mockConfig,
+          ToolErrorType.EXECUTION_DENIED,
+        );
+
+        expect(processStderrWriteSpy).toHaveBeenCalledWith(
+          expect.stringContaining(
+            'Warning: Tool "test-tool" requires user approval',
+          ),
+        );
+        expect(processStderrWriteSpy).toHaveBeenCalledWith(
+          expect.stringContaining('use the -y flag (YOLO mode)'),
+        );
+        expect(processExitSpy).not.toHaveBeenCalled();
+      });
+
+      it('should not show warning when EXECUTION_DENIED in interactive mode', () => {
+        (mockConfig.getDebugMode as Mock).mockReturnValue(false);
+        (mockConfig.isInteractive as Mock).mockReturnValue(true);
+        (
+          mockConfig.getOutputFormat as ReturnType<typeof vi.fn>
+        ).mockReturnValue(OutputFormat.TEXT);
+
+        handleToolError(
+          toolName,
+          toolError,
+          mockConfig,
+          ToolErrorType.EXECUTION_DENIED,
+        );
+
+        expect(processStderrWriteSpy).not.toHaveBeenCalled();
+        expect(processExitSpy).not.toHaveBeenCalled();
+      });
+
+      it('should not show warning when EXECUTION_DENIED in JSON mode', () => {
+        (mockConfig.getDebugMode as Mock).mockReturnValue(false);
+        (mockConfig.isInteractive as Mock).mockReturnValue(false);
+        (
+          mockConfig.getOutputFormat as ReturnType<typeof vi.fn>
+        ).mockReturnValue(OutputFormat.JSON);
+
+        handleToolError(
+          toolName,
+          toolError,
+          mockConfig,
+          ToolErrorType.EXECUTION_DENIED,
+        );
+
+        expect(processStderrWriteSpy).not.toHaveBeenCalled();
+        expect(processExitSpy).not.toHaveBeenCalled();
+      });
+
+      it('should not show warning for non-EXECUTION_DENIED errors', () => {
+        (mockConfig.getDebugMode as Mock).mockReturnValue(false);
+        (mockConfig.isInteractive as Mock).mockReturnValue(false);
+        (
+          mockConfig.getOutputFormat as ReturnType<typeof vi.fn>
+        ).mockReturnValue(OutputFormat.TEXT);
+
+        handleToolError(
+          toolName,
+          toolError,
+          mockConfig,
+          ToolErrorType.FILE_NOT_FOUND,
+        );
+
+        expect(processStderrWriteSpy).not.toHaveBeenCalled();
+        expect(processExitSpy).not.toHaveBeenCalled();
+      });
+    });
   });
 
   describe('handleCancellationError', () => {
diff --git a/packages/cli/src/utils/errors.ts b/packages/cli/src/utils/errors.ts
@@ -11,6 +11,7 @@ import {
   parseAndFormatApiError,
   FatalTurnLimitedError,
   FatalCancellationError,
+  ToolErrorType,
 } from '@qwen-code/qwen-code-core';
 
 export function getErrorMessage(error: unknown): string {
@@ -102,10 +103,24 @@ export function handleToolError(
   toolName: string,
   toolError: Error,
   config: Config,
-  _errorCode?: string | number,
+  errorCode?: string | number,
   resultDisplay?: string,
 ): void {
-  // Always just log to stderr; JSON/streaming formatting happens in the tool_result block elsewhere
+  // Check if this is a permission denied error in non-interactive mode
+  const isExecutionDenied = errorCode === ToolErrorType.EXECUTION_DENIED;
+  const isNonInteractive = !config.isInteractive();
+  const isTextMode = config.getOutputFormat() === OutputFormat.TEXT;
+
+  // Show warning for permission denied errors in non-interactive text mode
+  if (isExecutionDenied && isNonInteractive && isTextMode) {
+    const warningMessage =
+      `Warning: Tool "${toolName}" requires user approval but cannot execute in non-interactive mode.\n` +
+      `To enable automatic tool execution, use the -y flag (YOLO mode):\n` +
+      `Example: qwen -p 'your prompt' -y\n\n`;
+    process.stderr.write(warningMessage);
+  }
+
+  // Always log detailed error in debug mode
   if (config.getDebugMode()) {
     console.error(
       `Error executing tool ${toolName}: ${resultDisplay || toolError.message}`,
