diff --git a/packages/cli/src/ui/AppContainer.tsx b/packages/cli/src/ui/AppContainer.tsx index d1218058f..5e76bc194 100644 --- a/packages/cli/src/ui/AppContainer.tsx +++ b/packages/cli/src/ui/AppContainer.tsx @@ -555,7 +555,7 @@ export const AppContainer = (props: AppContainerProps) => { historyManager.addItem( { type: MessageType.INFO, - text: 'Refreshing hierarchical memory (GEMINI.md or other context files)...', + text: 'Refreshing hierarchical memory (QWEN.md or other context files)...', }, Date.now(), ); diff --git a/packages/core/src/core/openaiContentGenerator/converter.test.ts b/packages/core/src/core/openaiContentGenerator/converter.test.ts index 0bab4092c..888a65ad0 100644 --- a/packages/core/src/core/openaiContentGenerator/converter.test.ts +++ b/packages/core/src/core/openaiContentGenerator/converter.test.ts @@ -7,6 +7,7 @@ import { describe, it, expect, beforeEach } from 'vitest'; import { OpenAIContentConverter } from './converter.js'; import type { StreamingToolCallParser } from './streamingToolCallParser.js'; +import type { GenerateContentParameters, Content } from '@google/genai'; describe('OpenAIContentConverter', () => { let converter: OpenAIContentConverter; @@ -68,4 +69,77 @@ describe('OpenAIContentConverter', () => { expect(parser.getBuffer(0)).toBe(''); }); }); + + describe('convertGeminiRequestToOpenAI', () => { + const createRequestWithFunctionResponse = ( + response: Record, + ): GenerateContentParameters => { + const contents: Content[] = [ + { + role: 'model', + parts: [ + { + functionCall: { + id: 'call_1', + name: 'shell', + args: {}, + }, + }, + ], + }, + { + role: 'user', + parts: [ + { + functionResponse: { + id: 'call_1', + name: 'shell', + response, + }, + }, + ], + }, + ]; + return { + model: 'models/test', + contents, + }; + }; + + it('should extract raw output from function response objects', () => { + const request = createRequestWithFunctionResponse({ + output: 'Raw output text', + }); + + const messages = converter.convertGeminiRequestToOpenAI(request); + const toolMessage = messages.find((message) => message.role === 'tool'); + + expect(toolMessage).toBeDefined(); + expect(toolMessage?.content).toBe('Raw output text'); + }); + + it('should prioritize error field when present', () => { + const request = createRequestWithFunctionResponse({ + error: 'Command failed', + }); + + const messages = converter.convertGeminiRequestToOpenAI(request); + const toolMessage = messages.find((message) => message.role === 'tool'); + + expect(toolMessage).toBeDefined(); + expect(toolMessage?.content).toBe('Command failed'); + }); + + it('should stringify non-string responses', () => { + const request = createRequestWithFunctionResponse({ + data: { value: 42 }, + }); + + const messages = converter.convertGeminiRequestToOpenAI(request); + const toolMessage = messages.find((message) => message.role === 'tool'); + + expect(toolMessage).toBeDefined(); + expect(toolMessage?.content).toBe('{"data":{"value":42}}'); + }); + }); }); diff --git a/packages/core/src/core/openaiContentGenerator/converter.ts b/packages/core/src/core/openaiContentGenerator/converter.ts index 8157fada9..7966f3845 100644 --- a/packages/core/src/core/openaiContentGenerator/converter.ts +++ b/packages/core/src/core/openaiContentGenerator/converter.ts @@ -276,10 +276,7 @@ export class OpenAIContentConverter { messages.push({ role: 'tool' as const, tool_call_id: funcResponse.id || '', - content: - typeof funcResponse.response === 'string' - ? funcResponse.response - : JSON.stringify(funcResponse.response), + content: this.extractFunctionResponseContent(funcResponse.response), }); } return; @@ -359,6 +356,36 @@ export class OpenAIContentConverter { return { textParts, functionCalls, functionResponses, mediaParts }; } + private extractFunctionResponseContent(response: unknown): string { + if (response === null || response === undefined) { + return ''; + } + + if (typeof response === 'string') { + return response; + } + + if (typeof response === 'object') { + const responseObject = response as Record; + const output = responseObject['output']; + if (typeof output === 'string') { + return output; + } + + const error = responseObject['error']; + if (typeof error === 'string') { + return error; + } + } + + try { + const serialized = JSON.stringify(response); + return serialized ?? String(response); + } catch { + return String(response); + } + } + /** * Determine media type from MIME type */ diff --git a/packages/core/src/tools/exitPlanMode.test.ts b/packages/core/src/tools/exitPlanMode.test.ts index 59248b5b3..cdae6bfdb 100644 --- a/packages/core/src/tools/exitPlanMode.test.ts +++ b/packages/core/src/tools/exitPlanMode.test.ts @@ -131,16 +131,14 @@ describe('ExitPlanModeTool', () => { } const result = await invocation.execute(signal); - const expectedLlmMessage = - 'User has approved your plan. You can now start coding. Start with updating your todo list if applicable.'; - expect(result).toEqual({ - llmContent: expectedLlmMessage, - returnDisplay: { - type: 'plan_summary', - message: 'User approved the plan.', - plan: params.plan, - }, + expect(result.llmContent).toContain( + 'User has approved your plan. You can now start coding', + ); + expect(result.returnDisplay).toEqual({ + type: 'plan_summary', + message: 'User approved the plan.', + plan: params.plan, }); expect(mockConfig.setApprovalMode).toHaveBeenCalledWith( @@ -188,15 +186,12 @@ describe('ExitPlanModeTool', () => { const result = await invocation.execute(signal); - expect(result).toEqual({ - llmContent: JSON.stringify({ - success: false, - plan: params.plan, - error: 'Plan execution was not approved. Remaining in plan mode.', - }), - returnDisplay: - 'Plan execution was not approved. Remaining in plan mode.', - }); + expect(result.llmContent).toBe( + 'Plan execution was not approved. Remaining in plan mode.', + ); + expect(result.returnDisplay).toBe( + 'Plan execution was not approved. Remaining in plan mode.', + ); expect(mockConfig.setApprovalMode).toHaveBeenCalledWith( ApprovalMode.PLAN, @@ -215,50 +210,6 @@ describe('ExitPlanModeTool', () => { ); }); - it('should handle execution errors gracefully', async () => { - const params: ExitPlanModeParams = { - plan: 'Test plan', - }; - - const invocation = tool.build(params); - const confirmation = await invocation.shouldConfirmExecute( - new AbortController().signal, - ); - if (confirmation) { - // Don't approve the plan so we go through the rejection path - await confirmation.onConfirm(ToolConfirmationOutcome.Cancel); - } - - // Create a spy to simulate an error during the execution - const consoleSpy = vi - .spyOn(console, 'error') - .mockImplementation(() => {}); - - // Mock JSON.stringify to throw an error in the rejection path - const originalStringify = JSON.stringify; - vi.spyOn(JSON, 'stringify').mockImplementationOnce(() => { - throw new Error('JSON stringify error'); - }); - - const result = await invocation.execute(new AbortController().signal); - - expect(result).toEqual({ - llmContent: JSON.stringify({ - success: false, - error: 'Failed to present plan. Detail: JSON stringify error', - }), - returnDisplay: 'Error presenting plan: JSON stringify error', - }); - - expect(consoleSpy).toHaveBeenCalledWith( - '[ExitPlanModeTool] Error executing exit_plan_mode: JSON stringify error', - ); - - // Restore original JSON.stringify - JSON.stringify = originalStringify; - consoleSpy.mockRestore(); - }); - it('should return empty tool locations', () => { const params: ExitPlanModeParams = { plan: 'Test plan', diff --git a/packages/core/src/tools/exitPlanMode.ts b/packages/core/src/tools/exitPlanMode.ts index 4143d12f2..d39b8bbe3 100644 --- a/packages/core/src/tools/exitPlanMode.ts +++ b/packages/core/src/tools/exitPlanMode.ts @@ -115,17 +115,12 @@ class ExitPlanModeToolInvocation extends BaseToolInvocation< const rejectionMessage = 'Plan execution was not approved. Remaining in plan mode.'; return { - llmContent: JSON.stringify({ - success: false, - plan, - error: rejectionMessage, - }), + llmContent: rejectionMessage, returnDisplay: rejectionMessage, }; } - const llmMessage = - 'User has approved your plan. You can now start coding. Start with updating your todo list if applicable.'; + const llmMessage = `User has approved your plan. You can now start coding. Start with updating your todo list if applicable.`; const displayMessage = 'User approved the plan.'; return { @@ -142,11 +137,11 @@ class ExitPlanModeToolInvocation extends BaseToolInvocation< console.error( `[ExitPlanModeTool] Error executing exit_plan_mode: ${errorMessage}`, ); + + const errorLlmContent = `Failed to present plan: ${errorMessage}`; + return { - llmContent: JSON.stringify({ - success: false, - error: `Failed to present plan. Detail: ${errorMessage}`, - }), + llmContent: errorLlmContent, returnDisplay: `Error presenting plan: ${errorMessage}`, }; } diff --git a/packages/core/src/tools/memoryTool.test.ts b/packages/core/src/tools/memoryTool.test.ts index 3ed784fcb..471f87e0f 100644 --- a/packages/core/src/tools/memoryTool.test.ts +++ b/packages/core/src/tools/memoryTool.test.ts @@ -241,9 +241,7 @@ describe('MemoryTool', () => { expectedFsArgument, ); const successMessage = `Okay, I've remembered that in global memory: "${params.fact}"`; - expect(result.llmContent).toBe( - JSON.stringify({ success: true, message: successMessage }), - ); + expect(result.llmContent).toBe(successMessage); expect(result.returnDisplay).toBe(successMessage); }); @@ -271,9 +269,7 @@ describe('MemoryTool', () => { expectedFsArgument, ); const successMessage = `Okay, I've remembered that in project memory: "${params.fact}"`; - expect(result.llmContent).toBe( - JSON.stringify({ success: true, message: successMessage }), - ); + expect(result.llmContent).toBe(successMessage); expect(result.returnDisplay).toBe(successMessage); }); @@ -298,10 +294,7 @@ describe('MemoryTool', () => { const result = await invocation.execute(mockAbortSignal); expect(result.llmContent).toBe( - JSON.stringify({ - success: false, - error: `Failed to save memory. Detail: ${underlyingError.message}`, - }), + `Error saving memory: ${underlyingError.message}`, ); expect(result.returnDisplay).toBe( `Error saving memory: ${underlyingError.message}`, @@ -319,6 +312,8 @@ describe('MemoryTool', () => { expect(result.llmContent).toContain( 'Please specify where to save this memory', ); + expect(result.llmContent).toContain('Global:'); + expect(result.llmContent).toContain('Project:'); expect(result.returnDisplay).toContain('Global:'); expect(result.returnDisplay).toContain('Project:'); }); diff --git a/packages/core/src/tools/memoryTool.ts b/packages/core/src/tools/memoryTool.ts index 45829c014..42c4a6616 100644 --- a/packages/core/src/tools/memoryTool.ts +++ b/packages/core/src/tools/memoryTool.ts @@ -309,7 +309,7 @@ Preview of changes to be made to GLOBAL memory: if (!fact || typeof fact !== 'string' || fact.trim() === '') { const errorMessage = 'Parameter "fact" must be a non-empty string.'; return { - llmContent: JSON.stringify({ success: false, error: errorMessage }), + llmContent: `Error: ${errorMessage}`, returnDisplay: `Error: ${errorMessage}`, }; } @@ -324,10 +324,7 @@ Global: ${globalPath} (shared across all projects) Project: ${projectPath} (current project only)`; return { - llmContent: JSON.stringify({ - success: false, - error: 'Please specify where to save this memory', - }), + llmContent: errorMessage, returnDisplay: errorMessage, }; } @@ -344,10 +341,7 @@ Project: ${projectPath} (current project only)`; await fs.writeFile(memoryFilePath, modified_content, 'utf-8'); const successMessage = `Okay, I've updated the ${scope} memory file with your modifications.`; return { - llmContent: JSON.stringify({ - success: true, - message: successMessage, - }), + llmContent: successMessage, returnDisplay: successMessage, }; } else { @@ -359,10 +353,7 @@ Project: ${projectPath} (current project only)`; }); const successMessage = `Okay, I've remembered that in ${scope} memory: "${fact}"`; return { - llmContent: JSON.stringify({ - success: true, - message: successMessage, - }), + llmContent: successMessage, returnDisplay: successMessage, }; } @@ -372,11 +363,9 @@ Project: ${projectPath} (current project only)`; console.error( `[MemoryTool] Error executing save_memory for fact "${fact}" in ${scope}: ${errorMessage}`, ); + return { - llmContent: JSON.stringify({ - success: false, - error: `Failed to save memory. Detail: ${errorMessage}`, - }), + llmContent: `Error saving memory: ${errorMessage}`, returnDisplay: `Error saving memory: ${errorMessage}`, error: { message: errorMessage, diff --git a/packages/core/src/tools/todoWrite.test.ts b/packages/core/src/tools/todoWrite.test.ts index d142c5e06..cd21a55b7 100644 --- a/packages/core/src/tools/todoWrite.test.ts +++ b/packages/core/src/tools/todoWrite.test.ts @@ -141,7 +141,12 @@ describe('TodoWriteTool', () => { const invocation = tool.build(params); const result = await invocation.execute(mockAbortSignal); - expect(result.llmContent).toContain('success'); + expect(result.llmContent).toContain( + 'Todos have been modified successfully', + ); + expect(result.llmContent).toContain(''); + expect(result.llmContent).toContain('Your todo list has changed'); + expect(result.llmContent).toContain(JSON.stringify(params.todos)); expect(result.returnDisplay).toEqual({ type: 'todo_list', todos: [ @@ -178,7 +183,12 @@ describe('TodoWriteTool', () => { const invocation = tool.build(params); const result = await invocation.execute(mockAbortSignal); - expect(result.llmContent).toContain('success'); + expect(result.llmContent).toContain( + 'Todos have been modified successfully', + ); + expect(result.llmContent).toContain(''); + expect(result.llmContent).toContain('Your todo list has changed'); + expect(result.llmContent).toContain(JSON.stringify(params.todos)); expect(result.returnDisplay).toEqual({ type: 'todo_list', todos: [ @@ -208,7 +218,10 @@ describe('TodoWriteTool', () => { const invocation = tool.build(params); const result = await invocation.execute(mockAbortSignal); - expect(result.llmContent).toContain('"success":false'); + expect(result.llmContent).toContain('Failed to modify todos'); + expect(result.llmContent).toContain(''); + expect(result.llmContent).toContain('Todo list modification failed'); + expect(result.llmContent).toContain('Write failed'); expect(result.returnDisplay).toContain('Error writing todos'); }); @@ -223,7 +236,10 @@ describe('TodoWriteTool', () => { const invocation = tool.build(params); const result = await invocation.execute(mockAbortSignal); - expect(result.llmContent).toContain('success'); + expect(result.llmContent).toContain('Todo list has been cleared'); + expect(result.llmContent).toContain(''); + expect(result.llmContent).toContain('Your todo list is now empty'); + expect(result.llmContent).toContain('no pending tasks'); expect(result.returnDisplay).toEqual({ type: 'todo_list', todos: [], diff --git a/packages/core/src/tools/todoWrite.ts b/packages/core/src/tools/todoWrite.ts index 6201f151a..ffd0bcbb6 100644 --- a/packages/core/src/tools/todoWrite.ts +++ b/packages/core/src/tools/todoWrite.ts @@ -340,11 +340,30 @@ class TodoWriteToolInvocation extends BaseToolInvocation< todos: finalTodos, }; + // Create plain string format with system reminder + const todosJson = JSON.stringify(finalTodos); + let llmContent: string; + + if (finalTodos.length === 0) { + // Special message for empty todos + llmContent = `Todo list has been cleared. + + +Your todo list is now empty. DO NOT mention this explicitly to the user. You have no pending tasks in your todo list. +`; + } else { + // Normal message for todos with items + llmContent = `Todos have been modified successfully. Ensure that you continue to use the todo list to track your progress. Please proceed with the current tasks if applicable + + +Your todo list has changed. DO NOT mention this explicitly to the user. Here are the latest contents of your todo list: + +${todosJson}. Continue on with the tasks at hand if applicable. +`; + } + return { - llmContent: JSON.stringify({ - success: true, - todos: finalTodos, - }), + llmContent, returnDisplay: todoResultDisplay, }; } catch (error) { @@ -353,11 +372,16 @@ class TodoWriteToolInvocation extends BaseToolInvocation< console.error( `[TodoWriteTool] Error executing todo_write: ${errorMessage}`, ); + + // Create plain string format for error with system reminder + const errorLlmContent = `Failed to modify todos. An error occurred during the operation. + + +Todo list modification failed with error: ${errorMessage}. You may need to retry or handle this error appropriately. +`; + return { - llmContent: JSON.stringify({ - success: false, - error: `Failed to write todos. Detail: ${errorMessage}`, - }), + llmContent: errorLlmContent, returnDisplay: `Error writing todos: ${errorMessage}`, }; }