From 38ea6e1c74614ff972ca26743f5ba898c9f51f54 Mon Sep 17 00:00:00 2001 From: "mingholy.lmh" Date: Thu, 6 Nov 2025 15:26:44 +0800 Subject: [PATCH] fix: emit subagent user message and correct systemMessage properties --- .../io/BaseJsonOutputAdapter.test.ts | 108 ++++++++++++++++-- .../io/BaseJsonOutputAdapter.ts | 64 ++++++++++- .../io/JsonOutputAdapter.test.ts | 9 +- .../io/StreamJsonOutputAdapter.test.ts | 9 +- .../src/utils/nonInteractiveHelpers.test.ts | 19 ++- .../cli/src/utils/nonInteractiveHelpers.ts | 45 ++++++-- 6 files changed, 231 insertions(+), 23 deletions(-) diff --git a/packages/cli/src/nonInteractive/io/BaseJsonOutputAdapter.test.ts b/packages/cli/src/nonInteractive/io/BaseJsonOutputAdapter.test.ts index 6cbbea0d..0ba94cbb 100644 --- a/packages/cli/src/nonInteractive/io/BaseJsonOutputAdapter.test.ts +++ b/packages/cli/src/nonInteractive/io/BaseJsonOutputAdapter.test.ts @@ -23,6 +23,7 @@ import { type MessageState, type ResultOptions, partsToString, + partsToContentBlock, toolResultContent, extractTextFromBlocks, createExtendedUsage, @@ -853,7 +854,7 @@ describe('BaseJsonOutputAdapter', () => { }); describe('emitUserMessage', () => { - it('should emit user message', () => { + it('should emit user message with ContentBlock array', () => { const parts: Part[] = [{ text: 'Hello user' }]; adapter.emitUserMessage(parts); @@ -862,23 +863,34 @@ describe('BaseJsonOutputAdapter', () => { const message = adapter.emittedMessages[0]; expect(message.type).toBe('user'); if (message.type === 'user') { - expect(message.message.content).toBe('Hello user'); + expect(Array.isArray(message.message.content)).toBe(true); + if (Array.isArray(message.message.content)) { + expect(message.message.content).toHaveLength(1); + expect(message.message.content[0]).toEqual({ + type: 'text', + text: 'Hello user', + }); + } expect(message.parent_tool_use_id).toBeNull(); } }); - it('should handle multiple parts', () => { + it('should handle multiple parts and merge into single text block', () => { const parts: Part[] = [{ text: 'Hello' }, { text: ' World' }]; adapter.emitUserMessage(parts); const message = adapter.emittedMessages[0]; - if (message.type === 'user') { - expect(message.message.content).toBe('Hello World'); + if (message.type === 'user' && Array.isArray(message.message.content)) { + expect(message.message.content).toHaveLength(1); + expect(message.message.content[0]).toEqual({ + type: 'text', + text: 'Hello World', + }); } }); - it('should handle non-text parts', () => { + it('should handle non-text parts by converting to text blocks', () => { const parts: Part[] = [ { text: 'Hello' }, { functionCall: { name: 'test' } }, @@ -887,8 +899,15 @@ describe('BaseJsonOutputAdapter', () => { adapter.emitUserMessage(parts); const message = adapter.emittedMessages[0]; - if (message.type === 'user') { - expect(message.message.content).toContain('Hello'); + if (message.type === 'user' && Array.isArray(message.message.content)) { + expect(message.message.content.length).toBeGreaterThan(0); + const textBlock = message.message.content.find( + (block) => block.type === 'text', + ); + expect(textBlock).toBeDefined(); + if (textBlock && textBlock.type === 'text') { + expect(textBlock.text).toContain('Hello'); + } } }); }); @@ -1324,6 +1343,79 @@ describe('BaseJsonOutputAdapter', () => { }); describe('helper functions', () => { + describe('partsToContentBlock', () => { + it('should convert text parts to TextBlock array', () => { + const parts: Part[] = [{ text: 'Hello' }, { text: ' World' }]; + + const result = partsToContentBlock(parts); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ + type: 'text', + text: 'Hello World', + }); + }); + + it('should handle functionResponse parts by extracting output', () => { + const parts: Part[] = [ + { text: 'Result: ' }, + { + functionResponse: { + name: 'test', + response: { output: 'function output' }, + }, + }, + ]; + + const result = partsToContentBlock(parts); + + expect(result).toHaveLength(1); + expect(result[0].type).toBe('text'); + if (result[0].type === 'text') { + expect(result[0].text).toBe('Result: function output'); + } + }); + + it('should handle non-text parts by converting to JSON string', () => { + const parts: Part[] = [ + { text: 'Hello' }, + { functionCall: { name: 'test' } }, + ]; + + const result = partsToContentBlock(parts); + + expect(result.length).toBeGreaterThan(0); + const textBlock = result.find((block) => block.type === 'text'); + expect(textBlock).toBeDefined(); + if (textBlock && textBlock.type === 'text') { + expect(textBlock.text).toContain('Hello'); + expect(textBlock.text).toContain('functionCall'); + } + }); + + it('should handle empty array', () => { + const result = partsToContentBlock([]); + + expect(result).toEqual([]); + }); + + it('should merge consecutive text parts into single block', () => { + const parts: Part[] = [ + { text: 'Part 1' }, + { text: 'Part 2' }, + { text: 'Part 3' }, + ]; + + const result = partsToContentBlock(parts); + + expect(result).toHaveLength(1); + expect(result[0]).toEqual({ + type: 'text', + text: 'Part 1Part 2Part 3', + }); + }); + }); + describe('partsToString', () => { it('should convert text parts to string', () => { const parts: Part[] = [{ text: 'Hello' }, { text: ' World' }]; diff --git a/packages/cli/src/nonInteractive/io/BaseJsonOutputAdapter.ts b/packages/cli/src/nonInteractive/io/BaseJsonOutputAdapter.ts index 01ade407..3968c5cc 100644 --- a/packages/cli/src/nonInteractive/io/BaseJsonOutputAdapter.ts +++ b/packages/cli/src/nonInteractive/io/BaseJsonOutputAdapter.ts @@ -71,7 +71,7 @@ export interface ResultOptions { */ export interface MessageEmitter { emitMessage(message: CLIMessage): void; - emitUserMessage(parts: Part[]): void; + emitUserMessage(parts: Part[], parentToolUseId?: string | null): void; emitToolResult( request: ToolCallRequestInfo, response: ToolCallResponseInfo, @@ -922,14 +922,15 @@ export abstract class BaseJsonOutputAdapter { /** * Emits a user message. * @param parts - Array of Part objects + * @param parentToolUseId - Optional parent tool use ID for subagent messages */ - emitUserMessage(parts: Part[]): void { - const content = partsToString(parts); + emitUserMessage(parts: Part[], parentToolUseId?: string | null): void { + const content = partsToContentBlock(parts); const message: CLIUserMessage = { type: 'user', uuid: randomUUID(), session_id: this.getSessionId(), - parent_tool_use_id: null, + parent_tool_use_id: parentToolUseId ?? null, message: { role: 'user', content, @@ -1100,8 +1101,63 @@ export abstract class BaseJsonOutputAdapter { } } +/** + * Converts Part array to ContentBlock array. + * Handles various Part types including text, functionResponse, and other types. + * For functionResponse parts, extracts the output content. + * For other non-text parts, converts them to text representation. + * + * @param parts - Array of Part objects + * @returns Array of ContentBlock objects (primarily TextBlock) + */ +export function partsToContentBlock(parts: Part[]): ContentBlock[] { + const blocks: ContentBlock[] = []; + let currentTextBlock: TextBlock | null = null; + + for (const part of parts) { + let textContent: string | null = null; + + // Handle text parts + if ('text' in part && typeof part.text === 'string') { + textContent = part.text; + } + // Handle functionResponse parts - extract output content + else if ('functionResponse' in part && part.functionResponse) { + const output = + part.functionResponse.response?.['output'] ?? + part.functionResponse.response?.['content'] ?? + ''; + textContent = + typeof output === 'string' ? output : JSON.stringify(output); + } + // Handle other part types - convert to JSON string + else { + textContent = JSON.stringify(part); + } + + // If we have text content, add it to the current text block or create a new one + if (textContent !== null && textContent.length > 0) { + if (currentTextBlock === null) { + currentTextBlock = { + type: 'text', + text: textContent, + }; + blocks.push(currentTextBlock); + } else { + // Append to existing text block + currentTextBlock.text += textContent; + } + } + } + + // Return blocks array, or empty array if no content + return blocks; +} + /** * Converts Part array to string representation. + * This is a legacy function kept for backward compatibility. + * For new code, prefer using partsToContentBlock. * * @param parts - Array of Part objects * @returns String representation diff --git a/packages/cli/src/nonInteractive/io/JsonOutputAdapter.test.ts b/packages/cli/src/nonInteractive/io/JsonOutputAdapter.test.ts index 2f244037..2f4c9e44 100644 --- a/packages/cli/src/nonInteractive/io/JsonOutputAdapter.test.ts +++ b/packages/cli/src/nonInteractive/io/JsonOutputAdapter.test.ts @@ -556,7 +556,14 @@ describe('JsonOutputAdapter', () => { ); expect(userMessage).toBeDefined(); - expect(userMessage.message.content).toBe('Hello user'); + expect(Array.isArray(userMessage.message.content)).toBe(true); + if (Array.isArray(userMessage.message.content)) { + expect(userMessage.message.content).toHaveLength(1); + expect(userMessage.message.content[0]).toEqual({ + type: 'text', + text: 'Hello user', + }); + } }); it('should handle parent_tool_use_id', () => { diff --git a/packages/cli/src/nonInteractive/io/StreamJsonOutputAdapter.test.ts b/packages/cli/src/nonInteractive/io/StreamJsonOutputAdapter.test.ts index f7719d03..d0bd2325 100644 --- a/packages/cli/src/nonInteractive/io/StreamJsonOutputAdapter.test.ts +++ b/packages/cli/src/nonInteractive/io/StreamJsonOutputAdapter.test.ts @@ -714,7 +714,14 @@ describe('StreamJsonOutputAdapter', () => { const parsed = JSON.parse(output); expect(parsed.type).toBe('user'); - expect(parsed.message.content).toBe('Hello user'); + expect(Array.isArray(parsed.message.content)).toBe(true); + if (Array.isArray(parsed.message.content)) { + expect(parsed.message.content).toHaveLength(1); + expect(parsed.message.content[0]).toEqual({ + type: 'text', + text: 'Hello user', + }); + } }); it('should handle parent_tool_use_id', () => { diff --git a/packages/cli/src/utils/nonInteractiveHelpers.test.ts b/packages/cli/src/utils/nonInteractiveHelpers.test.ts index 70df4e92..7a0c5fe1 100644 --- a/packages/cli/src/utils/nonInteractiveHelpers.test.ts +++ b/packages/cli/src/utils/nonInteractiveHelpers.test.ts @@ -11,7 +11,11 @@ import type { TaskResultDisplay, ToolCallResponseInfo, } from '@qwen-code/qwen-code-core'; -import { ToolErrorType } from '@qwen-code/qwen-code-core'; +import { + ToolErrorType, + MCPServerStatus, + getMCPServerStatus, +} from '@qwen-code/qwen-code-core'; import type { Part } from '@google/genai'; import type { CLIUserMessage, @@ -55,6 +59,15 @@ vi.mock('../ui/utils/computeStats.js', () => ({ }), })); +vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + getMCPServerStatus: vi.fn(), + }; +}); + describe('normalizePartList', () => { it('should return empty array for null input', () => { expect(normalizePartList(null)).toEqual([]); @@ -477,6 +490,10 @@ describe('buildSystemMessage', () => { let mockConfig: Config; beforeEach(() => { + vi.clearAllMocks(); + // Mock getMCPServerStatus to return CONNECTED by default + vi.mocked(getMCPServerStatus).mockReturnValue(MCPServerStatus.CONNECTED); + mockConfig = { getToolRegistry: vi.fn().mockReturnValue({ getAllToolNames: vi.fn().mockReturnValue(['tool1', 'tool2']), diff --git a/packages/cli/src/utils/nonInteractiveHelpers.ts b/packages/cli/src/utils/nonInteractiveHelpers.ts index fe71730b..fe8fc528 100644 --- a/packages/cli/src/utils/nonInteractiveHelpers.ts +++ b/packages/cli/src/utils/nonInteractiveHelpers.ts @@ -13,7 +13,11 @@ import type { ToolCallResponseInfo, SessionMetrics, } from '@qwen-code/qwen-code-core'; -import { ToolErrorType } from '@qwen-code/qwen-code-core'; +import { + OutputFormat, + ToolErrorType, + getMCPServerStatus, +} from '@qwen-code/qwen-code-core'; import type { Part, PartListUnion } from '@google/genai'; import type { CLIUserMessage, @@ -243,13 +247,25 @@ export async function buildSystemMessage( const mcpServerList = mcpServers ? Object.keys(mcpServers).map((name) => ({ name, - status: 'connected', + status: getMCPServerStatus(name), })) : []; // Load slash commands const slashCommands = await loadSlashCommandNames(config); + // Load subagent names from config + let agentNames: string[] = []; + try { + const subagentManager = config.getSubagentManager(); + const subagents = await subagentManager.listSubagents(); + agentNames = subagents.map((subagent) => subagent.name); + } catch (error) { + if (config.getDebugMode()) { + console.error('[buildSystemMessage] Failed to load subagents:', error); + } + } + const systemMessage: CLISystemMessage = { type: 'system', subtype: 'init', @@ -261,13 +277,8 @@ export async function buildSystemMessage( model: config.getModel(), permissionMode, slash_commands: slashCommands, - apiKeySource: 'none', qwen_code_version: config.getCliVersion() || 'unknown', - output_style: 'default', - agents: [], - skills: [], - // Note: capabilities are NOT included in system messages - // They are only in the initialize control response + agents: agentNames, }; return systemMessage; @@ -536,11 +547,29 @@ export function createTaskToolProgressHandler( } } + // Handle subagent initial message (prompt) in non-interactive mode with json/stream-json output + // Emit when this is the first update (previous is undefined) and task starts + if ( + !previous && + taskDisplay.taskPrompt && + !config.isInteractive() && + (config.getOutputFormat() === OutputFormat.JSON || + config.getOutputFormat() === OutputFormat.STREAM_JSON) + ) { + // Emit the user message with the correct parent_tool_use_id + adapter.emitUserMessage( + [{ text: taskDisplay.taskPrompt }], + taskToolCallId, + ); + } + // Update previous state previousTaskStates.set(callId, taskDisplay); } }; + // No longer need to attach adapter to handler - task.ts uses TaskResultDisplay.message instead + return { handler: outputUpdateHandler, };