abbyswag/qwen-code
1
0
Fork 0
mirror of https://github.com/QwenLM/qwen-code.git synced 2025-12-21 09:17:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: subagent tool call messages

Browse Source
This commit is contained in:
mingholy.lmh
2025-11-04 20:29:38 +08:00
parent 14ad26f27e
commit a962e10406
14 changed files with 2011 additions and 1079 deletions
Download Patch File Download Diff File Expand all files Collapse all files

229
packages/cli/src/utils/errors.test.ts
View File

@@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { vi, type MockInstance } from 'vitest';
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 {
@@ -83,6 +83,7 @@ describe('errors', () => {
mockConfig = {
getOutputFormat: vi.fn().mockReturnValue(OutputFormat.TEXT),
getContentGeneratorConfig: vi.fn().mockReturnValue({ authType: 'test' }),
getDebugMode: vi.fn().mockReturnValue(true),
} as unknown as Config;
});
@@ -254,110 +255,180 @@ describe('errors', () => {
const toolName = 'test-tool';
const toolError = new Error('Tool failed');
describe('in text mode', () => {
describe('when debug mode is enabled', () => {
beforeEach(() => {
(
mockConfig.getOutputFormat as ReturnType<typeof vi.fn>
).mockReturnValue(OutputFormat.TEXT);
(mockConfig.getDebugMode as Mock).mockReturnValue(true);
});
it('should log error message to stderr', () => {
handleToolError(toolName, toolError, mockConfig);
describe('in text mode', () => {
beforeEach(() => {
(
mockConfig.getOutputFormat as ReturnType<typeof vi.fn>
).mockReturnValue(OutputFormat.TEXT);
});
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Error executing tool test-tool: Tool failed',
);
it('should log error message to stderr and not exit', () => {
handleToolError(toolName, toolError, mockConfig);
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Error executing tool test-tool: Tool failed',
);
expect(processExitSpy).not.toHaveBeenCalled();
});
it('should use resultDisplay when provided and not exit', () => {
handleToolError(
toolName,
toolError,
mockConfig,
'CUSTOM_ERROR',
'Custom display message',
);
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Error executing tool test-tool: Custom display message',
);
expect(processExitSpy).not.toHaveBeenCalled();
});
});
it('should use resultDisplay when provided', () => {
handleToolError(
toolName,
toolError,
mockConfig,
'CUSTOM_ERROR',
'Custom display message',
);
describe('in JSON mode', () => {
beforeEach(() => {
(
mockConfig.getOutputFormat as ReturnType<typeof vi.fn>
).mockReturnValue(OutputFormat.JSON);
});
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Error executing tool test-tool: Custom display message',
);
it('should log error message to stderr and not exit', () => {
handleToolError(toolName, toolError, mockConfig);
// In JSON mode, should not exit (just log to stderr when debug mode is on)
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Error executing tool test-tool: Tool failed',
);
expect(processExitSpy).not.toHaveBeenCalled();
});
it('should log error with custom error code and not exit', () => {
handleToolError(toolName, toolError, mockConfig, 'CUSTOM_TOOL_ERROR');
// In JSON mode, should not exit (just log to stderr when debug mode is on)
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Error executing tool test-tool: Tool failed',
);
expect(processExitSpy).not.toHaveBeenCalled();
});
it('should log error with numeric error code and not exit', () => {
handleToolError(toolName, toolError, mockConfig, 500);
// In JSON mode, should not exit (just log to stderr when debug mode is on)
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Error executing tool test-tool: Tool failed',
);
expect(processExitSpy).not.toHaveBeenCalled();
});
it('should prefer resultDisplay over error message and not exit', () => {
handleToolError(
toolName,
toolError,
mockConfig,
'DISPLAY_ERROR',
'Display message',
);
// In JSON mode, should not exit (just log to stderr when debug mode is on)
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Error executing tool test-tool: Display message',
);
expect(processExitSpy).not.toHaveBeenCalled();
});
});
describe('in STREAM_JSON mode', () => {
beforeEach(() => {
(
mockConfig.getOutputFormat as ReturnType<typeof vi.fn>
).mockReturnValue(OutputFormat.STREAM_JSON);
});
it('should log error message to stderr and not exit', () => {
handleToolError(toolName, toolError, mockConfig);
// Should not exit in STREAM_JSON mode (just log to stderr when debug mode is on)
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Error executing tool test-tool: Tool failed',
);
expect(processExitSpy).not.toHaveBeenCalled();
});
});
});
describe('in JSON mode', () => {
describe('when debug mode is disabled', () => {
beforeEach(() => {
(mockConfig.getDebugMode as Mock).mockReturnValue(false);
});
it('should not log and not exit in text mode', () => {
(
mockConfig.getOutputFormat as ReturnType<typeof vi.fn>
).mockReturnValue(OutputFormat.TEXT);
handleToolError(toolName, toolError, mockConfig);
expect(consoleErrorSpy).not.toHaveBeenCalled();
expect(processExitSpy).not.toHaveBeenCalled();
});
it('should not log and not exit in JSON mode', () => {
(
mockConfig.getOutputFormat as ReturnType<typeof vi.fn>
).mockReturnValue(OutputFormat.JSON);
});
it('should log error message to stderr and not exit', () => {
handleToolError(toolName, toolError, mockConfig);
// In JSON mode, should not exit (just log to stderr)
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Error executing tool test-tool: Tool failed',
);
expect(consoleErrorSpy).not.toHaveBeenCalled();
expect(processExitSpy).not.toHaveBeenCalled();
});
it('should log error with custom error code and not exit', () => {
handleToolError(toolName, toolError, mockConfig, 'CUSTOM_TOOL_ERROR');
// In JSON mode, should not exit (just log to stderr)
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Error executing tool test-tool: Tool failed',
);
expect(processExitSpy).not.toHaveBeenCalled();
});
it('should log error with numeric error code and not exit', () => {
handleToolError(toolName, toolError, mockConfig, 500);
// In JSON mode, should not exit (just log to stderr)
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Error executing tool test-tool: Tool failed',
);
expect(processExitSpy).not.toHaveBeenCalled();
});
it('should prefer resultDisplay over error message', () => {
handleToolError(
toolName,
toolError,
mockConfig,
'DISPLAY_ERROR',
'Display message',
);
// In JSON mode, should not exit (just log to stderr)
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Error executing tool test-tool: Display message',
);
expect(processExitSpy).not.toHaveBeenCalled();
});
it('should not exit in JSON mode', () => {
handleToolError(toolName, toolError, mockConfig);
// Should not throw (no exit)
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Error executing tool test-tool: Tool failed',
);
expect(processExitSpy).not.toHaveBeenCalled();
});
it('should not exit in STREAM_JSON mode', () => {
it('should not log and not exit in STREAM_JSON mode', () => {
(
mockConfig.getOutputFormat as ReturnType<typeof vi.fn>
).mockReturnValue(OutputFormat.STREAM_JSON);
handleToolError(toolName, toolError, mockConfig);
// Should not exit in STREAM_JSON mode
expect(consoleErrorSpy).toHaveBeenCalledWith(
'Error executing tool test-tool: Tool failed',
);
expect(consoleErrorSpy).not.toHaveBeenCalled();
expect(processExitSpy).not.toHaveBeenCalled();
});
});
describe('process exit behavior', () => {
beforeEach(() => {
(mockConfig.getDebugMode as Mock).mockReturnValue(true);
});
it('should never exit regardless of output format', () => {
// Test in TEXT mode
(
mockConfig.getOutputFormat as ReturnType<typeof vi.fn>
).mockReturnValue(OutputFormat.TEXT);
handleToolError(toolName, toolError, mockConfig);
expect(processExitSpy).not.toHaveBeenCalled();
// Test in JSON mode
(
mockConfig.getOutputFormat as ReturnType<typeof vi.fn>
).mockReturnValue(OutputFormat.JSON);
handleToolError(toolName, toolError, mockConfig);
expect(processExitSpy).not.toHaveBeenCalled();
// Test in STREAM_JSON mode
(
mockConfig.getOutputFormat as ReturnType<typeof vi.fn>
).mockReturnValue(OutputFormat.STREAM_JSON);
handleToolError(toolName, toolError, mockConfig);
expect(processExitSpy).not.toHaveBeenCalled();
});
});

332
packages/cli/src/utils/nonInteractiveHelpers.ts
View File

@@ -4,7 +4,15 @@
* SPDX-License-Identifier: Apache-2.0
*/
import type { Config } from '@qwen-code/qwen-code-core';
import type {
Config,
ToolResultDisplay,
TaskResultDisplay,
OutputUpdateHandler,
ToolCallRequestInfo,
ToolCallResponseInfo,
} from '@qwen-code/qwen-code-core';
import { ToolErrorType } from '@qwen-code/qwen-code-core';
import type { Part, PartListUnion } from '@google/genai';
import type {
CLIUserMessage,
@@ -15,6 +23,7 @@ import type {
} from '../nonInteractive/types.js';
import { CommandService } from '../services/CommandService.js';
import { BuiltinCommandLoader } from '../services/BuiltinCommandLoader.js';
import type { JsonOutputAdapterInterface } from '../nonInteractive/io/BaseJsonOutputAdapter.js';
/**
* Normalizes various part list formats into a consistent Part[] array.
@@ -244,3 +253,324 @@ export async function buildSystemMessage(
return systemMessage;
}
/**
* Creates an output update handler specifically for Task tool subagent execution.
* This handler monitors TaskResultDisplay updates and converts them to protocol messages
* using the unified adapter's subagent APIs. All emitted messages will have parent_tool_use_id set to
* the task tool's callId.
*
* @param config - Config instance for getting output format
* @param taskToolCallId - The task tool's callId to use as parent_tool_use_id for all subagent messages
* @param adapter - The unified adapter instance (JsonOutputAdapter or StreamJsonOutputAdapter)
* @returns An object containing the output update handler
*/
export function createTaskToolProgressHandler(
config: Config,
taskToolCallId: string,
adapter: JsonOutputAdapterInterface | undefined,
): {
handler: OutputUpdateHandler;
} {
// Track previous TaskResultDisplay states per tool call to detect changes
const previousTaskStates = new Map<string, TaskResultDisplay>();
// Track which tool call IDs have already emitted tool_use to prevent duplicates
const emittedToolUseIds = new Set<string>();
// Track which tool call IDs have already emitted tool_result to prevent duplicates
const emittedToolResultIds = new Set<string>();
/**
* Builds a ToolCallRequestInfo object from a tool call.
*
* @param toolCall - The tool call information
* @returns ToolCallRequestInfo object
*/
const buildRequest = (
toolCall: NonNullable<TaskResultDisplay['toolCalls']>[number],
): ToolCallRequestInfo => ({
callId: toolCall.callId,
name: toolCall.name,
args: toolCall.args || {},
isClientInitiated: true,
prompt_id: '',
response_id: undefined,
});
/**
* Builds a ToolCallResponseInfo object from a tool call.
*
* @param toolCall - The tool call information
* @returns ToolCallResponseInfo object
*/
const buildResponse = (
toolCall: NonNullable<TaskResultDisplay['toolCalls']>[number],
): ToolCallResponseInfo => ({
callId: toolCall.callId,
error:
toolCall.status === 'failed'
? new Error(toolCall.error || 'Tool execution failed')
: undefined,
errorType:
toolCall.status === 'failed' ? ToolErrorType.EXECUTION_FAILED : undefined,
resultDisplay: toolCall.resultDisplay,
responseParts: toolCall.responseParts || [],
});
/**
* Checks if a tool call has result content that should be emitted.
*
* @param toolCall - The tool call information
* @returns True if the tool call has result content to emit
*/
const hasResultContent = (
toolCall: NonNullable<TaskResultDisplay['toolCalls']>[number],
): boolean => {
// Check resultDisplay string
if (
typeof toolCall.resultDisplay === 'string' &&
toolCall.resultDisplay.trim().length > 0
) {
return true;
}
// Check responseParts - only check existence, don't parse for performance
if (toolCall.responseParts && toolCall.responseParts.length > 0) {
return true;
}
// Failed status should always emit result
return toolCall.status === 'failed';
};
/**
* Emits tool_use for a tool call if it hasn't been emitted yet.
*
* @param toolCall - The tool call information
* @param fallbackStatus - Optional fallback status if toolCall.status should be overridden
*/
const emitToolUseIfNeeded = (
toolCall: NonNullable<TaskResultDisplay['toolCalls']>[number],
fallbackStatus?: 'executing' | 'awaiting_approval',
): void => {
if (emittedToolUseIds.has(toolCall.callId)) {
return;
}
const toolCallToEmit: NonNullable<TaskResultDisplay['toolCalls']>[number] =
fallbackStatus
? {
...toolCall,
status: fallbackStatus,
}
: toolCall;
if (
toolCallToEmit.status === 'executing' ||
toolCallToEmit.status === 'awaiting_approval'
) {
if (adapter?.processSubagentToolCall) {
adapter.processSubagentToolCall(toolCallToEmit, taskToolCallId);
emittedToolUseIds.add(toolCall.callId);
}
}
};
/**
* Emits tool_result for a tool call if it hasn't been emitted yet and has content.
*
* @param toolCall - The tool call information
*/
const emitToolResultIfNeeded = (
toolCall: NonNullable<TaskResultDisplay['toolCalls']>[number],
): void => {
if (emittedToolResultIds.has(toolCall.callId)) {
return;
}
if (!hasResultContent(toolCall)) {
return;
}
// Mark as emitted even if we skip, to prevent duplicate emits
emittedToolResultIds.add(toolCall.callId);
if (adapter) {
const request = buildRequest(toolCall);
const response = buildResponse(toolCall);
// For subagent tool results, we need to pass parentToolUseId
// The adapter implementations accept an optional parentToolUseId parameter
if (
'emitToolResult' in adapter &&
typeof adapter.emitToolResult === 'function'
) {
adapter.emitToolResult(request, response, taskToolCallId);
} else {
adapter.emitToolResult(request, response);
}
}
};
/**
* Processes a tool call, ensuring tool_use and tool_result are emitted exactly once.
*
* @param toolCall - The tool call information
* @param previousCall - The previous state of the tool call (if any)
*/
const processToolCall = (
toolCall: NonNullable<TaskResultDisplay['toolCalls']>[number],
previousCall?: NonNullable<TaskResultDisplay['toolCalls']>[number],
): void => {
const isCompleted =
toolCall.status === 'success' || toolCall.status === 'failed';
const isExecuting =
toolCall.status === 'executing' ||
toolCall.status === 'awaiting_approval';
const wasExecuting =
previousCall &&
(previousCall.status === 'executing' ||
previousCall.status === 'awaiting_approval');
// Emit tool_use if needed
if (isExecuting) {
// Normal case: tool call is executing or awaiting approval
emitToolUseIfNeeded(toolCall);
} else if (isCompleted && !emittedToolUseIds.has(toolCall.callId)) {
// Edge case: tool call appeared with result already (shouldn't happen normally,
// but handle it gracefully by emitting tool_use with 'executing' status first)
emitToolUseIfNeeded(toolCall, 'executing');
} else if (wasExecuting && isCompleted) {
// Status changed from executing to completed - ensure tool_use was emitted
emitToolUseIfNeeded(toolCall, 'executing');
}
// Emit tool_result if tool call is completed
if (isCompleted) {
emitToolResultIfNeeded(toolCall);
}
};
const outputUpdateHandler = (
callId: string,
outputChunk: ToolResultDisplay,
) => {
// Only process TaskResultDisplay (Task tool updates)
if (
typeof outputChunk === 'object' &&
outputChunk !== null &&
'type' in outputChunk &&
outputChunk.type === 'task_execution'
) {
const taskDisplay = outputChunk as TaskResultDisplay;
const previous = previousTaskStates.get(callId);
// If no adapter, just track state (for non-JSON modes)
if (!adapter) {
previousTaskStates.set(callId, taskDisplay);
return;
}
// Only process if adapter supports subagent APIs
if (
!adapter.processSubagentToolCall ||
!adapter.emitSubagentErrorResult
) {
previousTaskStates.set(callId, taskDisplay);
return;
}
if (taskDisplay.toolCalls) {
if (!previous || !previous.toolCalls) {
// First time seeing tool calls - process all initial ones
for (const toolCall of taskDisplay.toolCalls) {
processToolCall(toolCall);
}
} else {
// Compare with previous state to find new/changed tool calls
for (const toolCall of taskDisplay.toolCalls) {
const previousCall = previous.toolCalls.find(
(tc) => tc.callId === toolCall.callId,
);
processToolCall(toolCall, previousCall);
}
}
}
// Handle task-level errors (status: 'failed', 'cancelled')
if (
taskDisplay.status === 'failed' ||
taskDisplay.status === 'cancelled'
) {
const previousStatus = previous?.status;
// Only emit error result if status changed to failed/cancelled
if (
previousStatus !== 'failed' &&
previousStatus !== 'cancelled' &&
previousStatus !== undefined
) {
const errorMessage =
taskDisplay.terminateReason ||
(taskDisplay.status === 'cancelled'
? 'Task was cancelled'
: 'Task execution failed');
// Use subagent adapter's emitSubagentErrorResult method
adapter.emitSubagentErrorResult(errorMessage, 0, taskToolCallId);
}
}
// Update previous state
previousTaskStates.set(callId, taskDisplay);
}
};
return {
handler: outputUpdateHandler,
};
}
/**
* Converts function response parts to a string representation.
* Handles functionResponse parts specially by extracting their output content.
*
* @param parts - Array of Part objects to convert
* @returns String representation of the parts
*/
export function functionResponsePartsToString(parts: Part[]): string {
return parts
.map((part) => {
if ('functionResponse' in part) {
const content = part.functionResponse?.response?.['output'] ?? '';
return content;
}
return JSON.stringify(part);
})
.join('');
}
/**
* Extracts content from a tool call response for inclusion in tool_result blocks.
* Uses functionResponsePartsToString to properly handle functionResponse parts,
* which correctly extracts output content from functionResponse objects rather
* than simply concatenating text or JSON.stringify.
*
* @param response - Tool call response information
* @returns String content for the tool_result block, or undefined if no content available
*/
export function toolResultContent(
response: ToolCallResponseInfo,
): string | undefined {
if (
typeof response.resultDisplay === 'string' &&
response.resultDisplay.trim().length > 0
) {
return response.resultDisplay;
}
if (response.responseParts && response.responseParts.length > 0) {
// Always use functionResponsePartsToString to properly handle
// functionResponse parts that contain output content
return functionResponsePartsToString(response.responseParts);
}
if (response.error) {
return response.error.message;
}
return undefined;
}