diff --git a/src/core/assistant-message/NativeToolCallParser.ts b/src/core/assistant-message/NativeToolCallParser.ts index df6619f405..5d156ac94d 100644 --- a/src/core/assistant-message/NativeToolCallParser.ts +++ b/src/core/assistant-message/NativeToolCallParser.ts @@ -479,11 +479,18 @@ export class NativeToolCallParser { if (!Array.isArray(coercedPartialFollowUp) && typeof coercedPartialFollowUp === "string") { try { const parsed = JSON.parse(coercedPartialFollowUp) - coercedPartialFollowUp = Array.isArray(parsed) ? parsed : undefined + coercedPartialFollowUp = Array.isArray(parsed) + ? parsed.map((s: any) => ({ text: s.text, mode: s.mode ?? null })) + : undefined } catch { coercedPartialFollowUp = undefined } - } else if (!Array.isArray(coercedPartialFollowUp)) { + } else if (Array.isArray(coercedPartialFollowUp)) { + coercedPartialFollowUp = coercedPartialFollowUp.map((s: any) => ({ + text: s.text, + mode: s.mode ?? null, + })) + } else { coercedPartialFollowUp = undefined } nativeArgs = { @@ -837,11 +844,18 @@ export class NativeToolCallParser { if (trimmed.length > 0) { try { const parsed = JSON.parse(trimmed) - coercedFinalFollowUp = Array.isArray(parsed) ? parsed : [{ text: trimmed }] + coercedFinalFollowUp = Array.isArray(parsed) + ? parsed.map((s: any) => ({ text: s.text, mode: s.mode ?? null })) + : [{ text: trimmed, mode: null }] } catch { - coercedFinalFollowUp = [{ text: trimmed }] + coercedFinalFollowUp = [{ text: trimmed, mode: null }] } } + } else if (Array.isArray(coercedFinalFollowUp)) { + coercedFinalFollowUp = coercedFinalFollowUp.map((s: any) => ({ + text: s.text, + mode: s.mode ?? null, + })) } nativeArgs = { question: args.question, diff --git a/src/core/tools/AskFollowupQuestionTool.ts b/src/core/tools/AskFollowupQuestionTool.ts index 93a539ae93..388b15a4b3 100644 --- a/src/core/tools/AskFollowupQuestionTool.ts +++ b/src/core/tools/AskFollowupQuestionTool.ts @@ -6,7 +6,7 @@ import { BaseTool, ToolCallbacks } from "./BaseTool" interface Suggestion { text: string - mode?: string + mode?: string | null } interface AskFollowupQuestionParams { @@ -27,7 +27,7 @@ interface AskFollowupQuestionParams { */ export function coerceFollowUp(value: unknown): Suggestion[] | undefined { if (Array.isArray(value)) { - return value + return normalizeSuggestions(value) } if (typeof value === "string" && value.trim().length > 0) { @@ -35,19 +35,31 @@ export function coerceFollowUp(value: unknown): Suggestion[] | undefined { try { const parsed = JSON.parse(value) if (Array.isArray(parsed)) { - return parsed + return normalizeSuggestions(parsed) } } catch { // Not valid JSON -- fall through to plain-string wrapping } // Wrap plain string as a single suggestion - return [{ text: value }] + return [{ text: value, mode: null }] } return undefined } +/** + * Ensure every suggestion has an explicit `mode` field (defaulting to `null`). + * The tool schema requires `mode` on each item (`required: ["text", "mode"]`), + * and some model Jinja templates fail if a required field is absent. + */ +function normalizeSuggestions(items: Suggestion[]): Suggestion[] { + return items.map((s) => ({ + text: s.text, + mode: s.mode ?? null, + })) +} + export class AskFollowupQuestionTool extends BaseTool<"ask_followup_question"> { readonly name = "ask_followup_question" as const diff --git a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts index fcdaaf6428..6be52ea6a4 100644 --- a/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts +++ b/src/core/tools/__tests__/askFollowupQuestionTool.spec.ts @@ -45,7 +45,7 @@ describe("askFollowupQuestionTool", () => { expect(mockCline.ask).toHaveBeenCalledWith( "followup", - expect.stringContaining('"suggest":[{"answer":"Option 1"},{"answer":"Option 2"}]'), + expect.stringContaining('"suggest":[{"answer":"Option 1","mode":null},{"answer":"Option 2","mode":null}]'), false, ) }) @@ -105,7 +105,7 @@ describe("askFollowupQuestionTool", () => { expect(mockCline.ask).toHaveBeenCalledWith( "followup", expect.stringContaining( - '"suggest":[{"answer":"Regular option"},{"answer":"Plan architecture","mode":"architect"}]', + '"suggest":[{"answer":"Regular option","mode":null},{"answer":"Plan architecture","mode":"architect"}]', ), false, ) @@ -189,7 +189,7 @@ describe("askFollowupQuestionTool", () => { // Plain string should be coerced to [{ text: "not an array" }] expect(mockCline.ask).toHaveBeenCalledWith( "followup", - expect.stringContaining('"suggest":[{"answer":"not an array"}]'), + expect.stringContaining('"suggest":[{"answer":"not an array","mode":null}]'), false, ) }) @@ -217,7 +217,9 @@ describe("askFollowupQuestionTool", () => { // 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"}]'), + expect.stringContaining( + '"suggest":[{"answer":"Option A","mode":null},{"answer":"Option B","mode":"code"}]', + ), false, ) }) @@ -248,18 +250,32 @@ describe("askFollowupQuestionTool", () => { }) describe("coerceFollowUp helper", () => { - it("should return arrays as-is", () => { + it("should normalize arrays to include mode: null when missing", () => { const input = [{ text: "Option 1" }, { text: "Option 2" }] - expect(coerceFollowUp(input)).toEqual(input) + expect(coerceFollowUp(input)).toEqual([ + { text: "Option 1", mode: null }, + { text: "Option 2", mode: null }, + ]) + }) + + it("should preserve existing mode values in arrays", () => { + const input = [{ text: "Option 1", mode: "code" }, { text: "Option 2" }] + expect(coerceFollowUp(input)).toEqual([ + { text: "Option 1", mode: "code" }, + { text: "Option 2", mode: null }, + ]) }) 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" }]) + expect(coerceFollowUp(input)).toEqual([ + { text: "A", mode: null }, + { text: "B", mode: "code" }, + ]) }) - it("should wrap a plain string as a single suggestion", () => { - expect(coerceFollowUp("some option")).toEqual([{ text: "some option" }]) + it("should wrap a plain string as a single suggestion with mode: null", () => { + expect(coerceFollowUp("some option")).toEqual([{ text: "some option", mode: null }]) }) it("should return undefined for null", () => { @@ -278,9 +294,9 @@ describe("askFollowupQuestionTool", () => { expect(coerceFollowUp(" ")).toBeUndefined() }) - it("should wrap a JSON string that parses to a non-array as a suggestion", () => { + it("should wrap a JSON string that parses to a non-array as a suggestion with mode: null", () => { // A JSON string like '{"text":"hello"}' is valid JSON but not an array - expect(coerceFollowUp('{"text":"hello"}')).toEqual([{ text: '{"text":"hello"}' }]) + expect(coerceFollowUp('{"text":"hello"}')).toEqual([{ text: '{"text":"hello"}', mode: null }]) }) }) @@ -400,7 +416,7 @@ describe("askFollowupQuestionTool", () => { follow_up: Array<{ text: string; mode?: string }> } expect(nativeArgs.question).toBe("Pick one") - expect(nativeArgs.follow_up).toEqual([{ text: "Option A" }]) + expect(nativeArgs.follow_up).toEqual([{ text: "Option A", mode: null }]) } }) @@ -422,7 +438,10 @@ describe("askFollowupQuestionTool", () => { follow_up: Array<{ text: string; mode?: string }> } expect(nativeArgs.question).toBe("Pick one") - expect(nativeArgs.follow_up).toEqual([{ text: "A" }, { text: "B" }]) + expect(nativeArgs.follow_up).toEqual([ + { text: "A", mode: null }, + { text: "B", mode: null }, + ]) } }) })