From a6c48e90eac3b9075c2480a5116a7c71809b5a03 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Sat, 9 Aug 2025 18:30:16 +0000 Subject: [PATCH] test: update askFollowupQuestionTool tests to align with simplified tool description - Reorganized tests into clearer sections for new format vs backward compatibility - Added comprehensive error handling tests including missing parameters, partial tool use, and exception handling - Added integration tests to verify implementation matches documentation examples - Fixed mock setup to properly handle all test scenarios - Removed problematic XML parsing test as the parser is very lenient - All 14 tests now passing successfully --- .../__tests__/askFollowupQuestionTool.spec.ts | 558 +++++++++++++----- 1 file changed, 403 insertions(+), 155 deletions(-) diff --git a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts index 7d0e9d29c0..46ecaa197f 100644 --- a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts +++ b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts @@ -1,197 +1,445 @@ -import { describe, it, expect, vi } from "vitest" +import { describe, it, expect, vi, beforeEach } from "vitest" import { askFollowupQuestionTool } from "../askFollowupQuestionTool" import { ToolUse } from "../../../shared/tools" describe("askFollowupQuestionTool", () => { let mockCline: any let mockPushToolResult: any + let mockAskApproval: any + let mockHandleError: any + let mockRemoveClosingTag: any let toolResult: any beforeEach(() => { vi.clearAllMocks() mockCline = { - ask: vi.fn().mockResolvedValue({ text: "Test response" }), + ask: vi.fn().mockResolvedValue({ text: "Test response", images: [] }), say: vi.fn().mockResolvedValue(undefined), + sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing parameter error"), + recordToolError: vi.fn(), consecutiveMistakeCount: 0, } mockPushToolResult = vi.fn((result) => { toolResult = result }) + + mockAskApproval = vi.fn() + mockHandleError = vi.fn() + mockRemoveClosingTag = vi.fn((tag, content) => content) }) - it("should parse suggestions without mode (new nested format)", async () => { - const block: ToolUse = { - type: "tool_use", - name: "ask_followup_question", - params: { - question: "What would you like to do?", - follow_up: - "Option 1Option 2", - }, - partial: false, - } + describe("New nested XML format (current implementation)", () => { + it("should parse suggestions without mode", async () => { + const block: ToolUse = { + type: "tool_use", + name: "ask_followup_question", + params: { + question: "What is the path to the frontend-config.json file?", + follow_up: + "./src/frontend-config.json./config/frontend-config.json./frontend-config.json", + }, + partial: false, + } - await askFollowupQuestionTool( - mockCline, - block, - vi.fn(), - vi.fn(), - mockPushToolResult, - vi.fn((tag, content) => content), - ) + await askFollowupQuestionTool( + mockCline, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) - expect(mockCline.ask).toHaveBeenCalledWith( - "followup", - expect.stringContaining('"suggest":[{"answer":"Option 1"},{"answer":"Option 2"}]'), - false, - ) + expect(mockCline.ask).toHaveBeenCalledWith( + "followup", + expect.stringContaining( + '"suggest":[{"answer":"./src/frontend-config.json"},{"answer":"./config/frontend-config.json"},{"answer":"./frontend-config.json"}]', + ), + false, + ) + expect(mockCline.consecutiveMistakeCount).toBe(0) + }) + + it("should parse suggestions with mode", async () => { + const block: ToolUse = { + type: "tool_use", + name: "ask_followup_question", + params: { + question: "How would you like to proceed?", + follow_up: + "codeStart implementing the solutionarchitectPlan the architecture first", + }, + partial: false, + } + + await askFollowupQuestionTool( + mockCline, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockCline.ask).toHaveBeenCalledWith( + "followup", + expect.stringContaining( + '"suggest":[{"answer":"Start implementing the solution","mode":"code"},{"answer":"Plan the architecture first","mode":"architect"}]', + ), + false, + ) + expect(mockCline.consecutiveMistakeCount).toBe(0) + }) + + it("should handle mixed suggestions with and without mode", async () => { + const block: ToolUse = { + type: "tool_use", + name: "ask_followup_question", + params: { + question: "What would you like to do next?", + follow_up: + "Continue with current approachdebugDebug the issueSkip this step", + }, + partial: false, + } + + await askFollowupQuestionTool( + mockCline, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockCline.ask).toHaveBeenCalledWith( + "followup", + expect.stringContaining( + '"suggest":[{"answer":"Continue with current approach"},{"answer":"Debug the issue","mode":"debug"},{"answer":"Skip this step"}]', + ), + false, + ) + expect(mockCline.consecutiveMistakeCount).toBe(0) + }) + + it("should handle single suggestion", async () => { + const block: ToolUse = { + type: "tool_use", + name: "ask_followup_question", + params: { + question: "Should I proceed with the default configuration?", + follow_up: "Yes, use the default configuration", + }, + partial: false, + } + + await askFollowupQuestionTool( + mockCline, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockCline.ask).toHaveBeenCalledWith( + "followup", + expect.stringContaining('"suggest":[{"answer":"Yes, use the default configuration"}]'), + false, + ) + }) }) - it("should parse suggestions with mode (new nested format)", async () => { - const block: ToolUse = { - type: "tool_use", - name: "ask_followup_question", - params: { - question: "What would you like to do?", - follow_up: - "codeWrite codedebugDebug issue", - }, - partial: false, - } + describe("Backward compatibility with old format", () => { + it("should parse suggestions without mode attributes", async () => { + const block: ToolUse = { + type: "tool_use", + name: "ask_followup_question", + params: { + question: "What would you like to do?", + follow_up: "Option 1Option 2", + }, + partial: false, + } - await askFollowupQuestionTool( - mockCline, - block, - vi.fn(), - vi.fn(), - mockPushToolResult, - vi.fn((tag, content) => content), - ) + await askFollowupQuestionTool( + mockCline, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) - expect(mockCline.ask).toHaveBeenCalledWith( - "followup", - expect.stringContaining( - '"suggest":[{"answer":"Write code","mode":"code"},{"answer":"Debug issue","mode":"debug"}]', - ), - false, - ) + expect(mockCline.ask).toHaveBeenCalledWith( + "followup", + expect.stringContaining('"suggest":[{"answer":"Option 1"},{"answer":"Option 2"}]'), + false, + ) + }) + + it("should parse suggestions with mode attributes", async () => { + const block: ToolUse = { + type: "tool_use", + name: "ask_followup_question", + params: { + question: "What would you like to do?", + follow_up: 'Write codeDebug issue', + }, + partial: false, + } + + await askFollowupQuestionTool( + mockCline, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockCline.ask).toHaveBeenCalledWith( + "followup", + expect.stringContaining( + '"suggest":[{"answer":"Write code","mode":"code"},{"answer":"Debug issue","mode":"debug"}]', + ), + false, + ) + }) + + it("should handle mixed suggestions with and without mode attributes", async () => { + const block: ToolUse = { + type: "tool_use", + name: "ask_followup_question", + params: { + question: "What would you like to do?", + follow_up: 'Regular optionPlan architecture', + }, + partial: false, + } + + await askFollowupQuestionTool( + mockCline, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockCline.ask).toHaveBeenCalledWith( + "followup", + expect.stringContaining( + '"suggest":[{"answer":"Regular option"},{"answer":"Plan architecture","mode":"architect"}]', + ), + false, + ) + }) }) - it("should handle mixed suggestions with and without mode (new nested format)", async () => { - const block: ToolUse = { - type: "tool_use", - name: "ask_followup_question", - params: { - question: "What would you like to do?", - follow_up: - "Regular optionarchitectPlan architecture", - }, - partial: false, - } + describe("Error handling", () => { + it("should handle missing question parameter", async () => { + const block: ToolUse = { + type: "tool_use", + name: "ask_followup_question", + params: { + follow_up: "Option 1", + }, + partial: false, + } - await askFollowupQuestionTool( - mockCline, - block, - vi.fn(), - vi.fn(), - mockPushToolResult, - vi.fn((tag, content) => content), - ) + await askFollowupQuestionTool( + mockCline, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) - expect(mockCline.ask).toHaveBeenCalledWith( - "followup", - expect.stringContaining( - '"suggest":[{"answer":"Regular option"},{"answer":"Plan architecture","mode":"architect"}]', - ), - false, - ) + expect(mockCline.recordToolError).toHaveBeenCalledWith("ask_followup_question") + expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalledWith("ask_followup_question", "question") + expect(mockPushToolResult).toHaveBeenCalledWith("Missing parameter error") + expect(mockCline.consecutiveMistakeCount).toBe(1) + }) + + it("should handle partial tool use", async () => { + const block: ToolUse = { + type: "tool_use", + name: "ask_followup_question", + params: { + question: "Partial question?", + }, + partial: true, + } + + await askFollowupQuestionTool( + mockCline, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockCline.ask).toHaveBeenCalledWith("followup", "Partial question?", true) + expect(mockPushToolResult).not.toHaveBeenCalled() + }) + + it("should handle question without follow_up suggestions", async () => { + const block: ToolUse = { + type: "tool_use", + name: "ask_followup_question", + params: { + question: "What is your preference?", + }, + partial: false, + } + + await askFollowupQuestionTool( + mockCline, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockCline.ask).toHaveBeenCalledWith( + "followup", + expect.stringContaining('"question":"What is your preference?"'), + false, + ) + expect(mockCline.ask).toHaveBeenCalledWith("followup", expect.stringContaining('"suggest":[]'), false) + expect(mockCline.consecutiveMistakeCount).toBe(0) + }) + + it("should return user response with images", async () => { + const block: ToolUse = { + type: "tool_use", + name: "ask_followup_question", + params: { + question: "Can you show me the error?", + follow_up: "I'll paste the error message", + }, + partial: false, + } + + const mockImages = ["image1.png", "image2.png"] + mockCline.ask = vi.fn().mockResolvedValue({ text: "Here's the error screenshot", images: mockImages }) + + // Mock formatResponse.toolResult + const formatResponse = await import("../../prompts/responses") + vi.spyOn(formatResponse.formatResponse, "toolResult").mockReturnValue( + "\nHere's the error screenshot\n", + ) + + await askFollowupQuestionTool( + mockCline, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockCline.say).toHaveBeenCalledWith("user_feedback", "Here's the error screenshot", mockImages) + expect(formatResponse.formatResponse.toolResult).toHaveBeenCalledWith( + "\nHere's the error screenshot\n", + mockImages, + ) + expect(mockPushToolResult).toHaveBeenCalledWith("\nHere's the error screenshot\n") + }) + + it("should handle exception during tool execution", async () => { + const block: ToolUse = { + type: "tool_use", + name: "ask_followup_question", + params: { + question: "Test question", + follow_up: "Test", + }, + partial: false, + } + + const testError = new Error("Unexpected error") + mockCline.ask = vi.fn().mockRejectedValue(testError) + + await askFollowupQuestionTool( + mockCline, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockHandleError).toHaveBeenCalledWith("asking question", testError) + }) }) - // Backward compatibility tests for old format - it("should parse suggestions without mode attributes (old format - backward compatibility)", async () => { - const block: ToolUse = { - type: "tool_use", - name: "ask_followup_question", - params: { - question: "What would you like to do?", - follow_up: "Option 1Option 2", - }, - partial: false, - } + describe("Integration with tool description format", () => { + it("should handle format matching documentation example", async () => { + // This test ensures the implementation matches the exact format shown in the documentation + const block: ToolUse = { + type: "tool_use", + name: "ask_followup_question", + params: { + question: "What is the path to the frontend-config.json file?", + follow_up: `./src/frontend-config.json +./config/frontend-config.json +./frontend-config.json`, + }, + partial: false, + } - await askFollowupQuestionTool( - mockCline, - block, - vi.fn(), - vi.fn(), - mockPushToolResult, - vi.fn((tag, content) => content), - ) + await askFollowupQuestionTool( + mockCline, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) - expect(mockCline.ask).toHaveBeenCalledWith( - "followup", - expect.stringContaining('"suggest":[{"answer":"Option 1"},{"answer":"Option 2"}]'), - false, - ) - }) + const callArgs = mockCline.ask.mock.calls[0] + const parsedJson = JSON.parse(callArgs[1]) - it("should parse suggestions with mode attributes (old format - backward compatibility)", async () => { - const block: ToolUse = { - type: "tool_use", - name: "ask_followup_question", - params: { - question: "What would you like to do?", - follow_up: 'Write codeDebug issue', - }, - partial: false, - } + expect(parsedJson.question).toBe("What is the path to the frontend-config.json file?") + expect(parsedJson.suggest).toHaveLength(3) + expect(parsedJson.suggest[0]).toEqual({ answer: "./src/frontend-config.json" }) + expect(parsedJson.suggest[1]).toEqual({ answer: "./config/frontend-config.json" }) + expect(parsedJson.suggest[2]).toEqual({ answer: "./frontend-config.json" }) + }) - await askFollowupQuestionTool( - mockCline, - block, - vi.fn(), - vi.fn(), - mockPushToolResult, - vi.fn((tag, content) => content), - ) + it("should handle inline format with mode switching as per documentation", async () => { + const block: ToolUse = { + type: "tool_use", + name: "ask_followup_question", + params: { + question: "How would you like to proceed?", + follow_up: + "First suggestioncodeAction with mode switch", + }, + partial: false, + } - expect(mockCline.ask).toHaveBeenCalledWith( - "followup", - expect.stringContaining( - '"suggest":[{"answer":"Write code","mode":"code"},{"answer":"Debug issue","mode":"debug"}]', - ), - false, - ) - }) + await askFollowupQuestionTool( + mockCline, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) - it("should handle mixed suggestions with and without mode attributes (old format - backward compatibility)", async () => { - const block: ToolUse = { - type: "tool_use", - name: "ask_followup_question", - params: { - question: "What would you like to do?", - follow_up: 'Regular optionPlan architecture', - }, - partial: false, - } + const callArgs = mockCline.ask.mock.calls[0] + const parsedJson = JSON.parse(callArgs[1]) - await askFollowupQuestionTool( - mockCline, - block, - vi.fn(), - vi.fn(), - mockPushToolResult, - vi.fn((tag, content) => content), - ) - - expect(mockCline.ask).toHaveBeenCalledWith( - "followup", - expect.stringContaining( - '"suggest":[{"answer":"Regular option"},{"answer":"Plan architecture","mode":"architect"}]', - ), - false, - ) + expect(parsedJson.suggest[0]).toEqual({ answer: "First suggestion" }) + expect(parsedJson.suggest[1]).toEqual({ answer: "Action with mode switch", mode: "code" }) + }) }) })