diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index bda7c71eb8..df6619f405 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -475,9 +475,20 @@ export class NativeToolCallParser { case "ask_followup_question": if (partialArgs.question !== undefined || partialArgs.follow_up !== undefined) { + let coercedPartialFollowUp = partialArgs.follow_up + if (!Array.isArray(coercedPartialFollowUp) && typeof coercedPartialFollowUp === "string") { + try { + const parsed = JSON.parse(coercedPartialFollowUp) + coercedPartialFollowUp = Array.isArray(parsed) ? parsed : undefined + } catch { + coercedPartialFollowUp = undefined + } + } else if (!Array.isArray(coercedPartialFollowUp)) { + coercedPartialFollowUp = undefined + } nativeArgs = { question: partialArgs.question, - follow_up: Array.isArray(partialArgs.follow_up) ? partialArgs.follow_up : undefined, + follow_up: coercedPartialFollowUp, } } break @@ -820,9 +831,21 @@ export class NativeToolCallParser { case "ask_followup_question": if (args.question !== undefined && args.follow_up !== undefined) { + let coercedFinalFollowUp = args.follow_up + if (!Array.isArray(coercedFinalFollowUp) && typeof coercedFinalFollowUp === "string") { + const trimmed = (coercedFinalFollowUp as string).trim() + if (trimmed.length > 0) { + try { + const parsed = JSON.parse(trimmed) + coercedFinalFollowUp = Array.isArray(parsed) ? parsed : [{ text: trimmed }] + } catch { + coercedFinalFollowUp = [{ text: trimmed }] + } + } + } nativeArgs = { question: args.question, - follow_up: args.follow_up, + follow_up: coercedFinalFollowUp, } as NativeArgsFor } break diff --git a/src/core/tools/AskFollowupQuestionTool.ts b/src/core/tools/AskFollowupQuestionTool.ts index 22cdbcf5de..93a539ae93 100644 --- a/src/core/tools/AskFollowupQuestionTool.ts +++ b/src/core/tools/AskFollowupQuestionTool.ts @@ -14,11 +14,46 @@ interface AskFollowupQuestionParams { follow_up: Suggestion[] } +/** + * Coerce a follow_up value from various formats into the expected Suggestion array. + * Some models (e.g. smaller Qwen models) output follow_up as a string instead of an array. + * This helper normalizes the value so the tool works regardless of the model's output format. + * + * Supported coercions: + * - Already an array: returned as-is + * - A JSON string that parses to an array: parsed and returned + * - A plain string: wrapped as a single suggestion `[{ text: value }]` + * - Anything else (null, undefined, number, etc.): returns undefined so callers can error + */ +export function coerceFollowUp(value: unknown): Suggestion[] | undefined { + if (Array.isArray(value)) { + return value + } + + if (typeof value === "string" && value.trim().length > 0) { + // Try parsing as JSON first (model may have serialized the array as a string) + try { + const parsed = JSON.parse(value) + if (Array.isArray(parsed)) { + return parsed + } + } catch { + // Not valid JSON -- fall through to plain-string wrapping + } + + // Wrap plain string as a single suggestion + return [{ text: value }] + } + + return undefined +} + export class AskFollowupQuestionTool extends BaseTool<"ask_followup_question"> { readonly name = "ask_followup_question" as const async execute(params: AskFollowupQuestionParams, task: Task, callbacks: ToolCallbacks): Promise { - const { question, follow_up } = params + const { question } = params + const follow_up = coerceFollowUp(params.follow_up) const { handleError, pushToolResult } = callbacks const recordMissingParamError = async (paramName: string): Promise => { diff --git a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts index 63bfad8a3d..fcdaaf6428 100644 --- a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts +++ b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts @@ -1,4 +1,4 @@ -import { askFollowupQuestionTool } from "../AskFollowupQuestionTool" +import { askFollowupQuestionTool, coerceFollowUp } from "../AskFollowupQuestionTool" import { ToolUse } from "../../../shared/tools" import { NativeToolCallParser } from "../../assistant-message/NativeToolCallParser" @@ -166,7 +166,7 @@ describe("askFollowupQuestionTool", () => { expect(mockCline.ask).not.toHaveBeenCalled() }) - it("should handle non-array follow_up parameter", async () => { + it("should coerce a plain string follow_up into a single-item array", async () => { const block: ToolUse = { type: "tool_use", name: "ask_followup_question", @@ -186,14 +186,104 @@ describe("askFollowupQuestionTool", () => { pushToolResult: mockPushToolResult, }) + // Plain string should be coerced to [{ text: "not an array" }] + expect(mockCline.ask).toHaveBeenCalledWith( + "followup", + expect.stringContaining('"suggest":[{"answer":"not an array"}]'), + false, + ) + }) + + it("should coerce a JSON string array follow_up into a proper array", async () => { + const block: ToolUse = { + type: "tool_use", + name: "ask_followup_question", + params: { + question: "What would you like to do?", + }, + nativeArgs: { + question: "What would you like to do?", + follow_up: '[{"text":"Option A"},{"text":"Option B","mode":"code"}]' as any, + } as any, + partial: false, + } + + await askFollowupQuestionTool.handle(mockCline, block as ToolUse<"ask_followup_question">, { + askApproval: vi.fn(), + handleError: vi.fn(), + pushToolResult: mockPushToolResult, + }) + + // JSON string should be parsed into a proper array + expect(mockCline.ask).toHaveBeenCalledWith( + "followup", + expect.stringContaining('"suggest":[{"answer":"Option A"},{"answer":"Option B","mode":"code"}]'), + false, + ) + }) + + it("should handle number follow_up parameter as missing", async () => { + const block: ToolUse = { + type: "tool_use", + name: "ask_followup_question", + params: { + question: "What would you like to do?", + }, + nativeArgs: { + question: "What would you like to do?", + follow_up: 42 as any, + } as any, + partial: false, + } + + await askFollowupQuestionTool.handle(mockCline, block as ToolUse<"ask_followup_question">, { + askApproval: vi.fn(), + handleError: vi.fn(), + pushToolResult: mockPushToolResult, + }) + expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalledWith("ask_followup_question", "follow_up") - expect(mockCline.recordToolError).toHaveBeenCalledWith("ask_followup_question") - expect(mockCline.didToolFailInCurrentTurn).toBe(true) - expect(mockCline.consecutiveMistakeCount).toBe(1) expect(mockCline.ask).not.toHaveBeenCalled() }) }) + describe("coerceFollowUp helper", () => { + it("should return arrays as-is", () => { + const input = [{ text: "Option 1" }, { text: "Option 2" }] + expect(coerceFollowUp(input)).toEqual(input) + }) + + it("should parse a JSON string containing an array", () => { + const input = '[{"text":"A"},{"text":"B","mode":"code"}]' + expect(coerceFollowUp(input)).toEqual([{ text: "A" }, { text: "B", mode: "code" }]) + }) + + it("should wrap a plain string as a single suggestion", () => { + expect(coerceFollowUp("some option")).toEqual([{ text: "some option" }]) + }) + + it("should return undefined for null", () => { + expect(coerceFollowUp(null)).toBeUndefined() + }) + + it("should return undefined for undefined", () => { + expect(coerceFollowUp(undefined)).toBeUndefined() + }) + + it("should return undefined for empty string", () => { + expect(coerceFollowUp("")).toBeUndefined() + }) + + it("should return undefined for whitespace-only string", () => { + expect(coerceFollowUp(" ")).toBeUndefined() + }) + + it("should wrap a JSON string that parses to a non-array as a suggestion", () => { + // A JSON string like '{"text":"hello"}' is valid JSON but not an array + expect(coerceFollowUp('{"text":"hello"}')).toEqual([{ text: '{"text":"hello"}' }]) + }) + }) + describe("handlePartial with native protocol", () => { it("should only send question during partial streaming to avoid raw JSON display", async () => { const block: ToolUse<"ask_followup_question"> = { @@ -292,5 +382,48 @@ describe("askFollowupQuestionTool", () => { }) } }) + + it("should coerce string follow_up to array during finalization", () => { + NativeToolCallParser.startStreamingToolCall("call_789", "ask_followup_question") + + // Simulate a model that outputs follow_up as a plain string + const jsonWithStringFollowUp = '{"question":"Pick one","follow_up":"Option A"}' + NativeToolCallParser.processStreamingChunk("call_789", jsonWithStringFollowUp) + + const result = NativeToolCallParser.finalizeStreamingToolCall("call_789") + + expect(result).not.toBeNull() + expect(result?.type).toBe("tool_use") + if (result?.type === "tool_use") { + const nativeArgs = result.nativeArgs as { + question: string + follow_up: Array<{ text: string; mode?: string }> + } + expect(nativeArgs.question).toBe("Pick one") + expect(nativeArgs.follow_up).toEqual([{ text: "Option A" }]) + } + }) + + it("should coerce JSON-string follow_up to array during finalization", () => { + NativeToolCallParser.startStreamingToolCall("call_101", "ask_followup_question") + + // Simulate a model that outputs follow_up as a JSON string of an array + const jsonWithJsonStringFollowUp = + '{"question":"Pick one","follow_up":"[{\\"text\\":\\"A\\"},{\\"text\\":\\"B\\"}]"}' + NativeToolCallParser.processStreamingChunk("call_101", jsonWithJsonStringFollowUp) + + const result = NativeToolCallParser.finalizeStreamingToolCall("call_101") + + expect(result).not.toBeNull() + expect(result?.type).toBe("tool_use") + if (result?.type === "tool_use") { + const nativeArgs = result.nativeArgs as { + question: string + follow_up: Array<{ text: string; mode?: string }> + } + expect(nativeArgs.question).toBe("Pick one") + expect(nativeArgs.follow_up).toEqual([{ text: "A" }, { text: "B" }]) + } + }) }) })