mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-10-07 02:58:15 +00:00
fix: add follow_up param validation in AskFollowupQuestionTool (#11484)
Added strict validation for the `follow_up` parameter to check for presence and type (Array). Added test cases covering missing, null, and invalid type scenarios. Refactored parameter error handling into a helper method to reduce duplication.
This commit is contained in:
parent
6ef149d567
commit
77ed60a173
2 changed files with 98 additions and 4 deletions
|
|
@ -21,12 +21,21 @@ export class AskFollowupQuestionTool extends BaseTool<"ask_followup_question"> {
|
|||
const { question, follow_up } = params
|
||||
const { handleError, pushToolResult } = callbacks
|
||||
|
||||
const recordMissingParamError = async (paramName: string): Promise<void> => {
|
||||
task.consecutiveMistakeCount++
|
||||
task.recordToolError("ask_followup_question")
|
||||
task.didToolFailInCurrentTurn = true
|
||||
pushToolResult(await task.sayAndCreateMissingParamError("ask_followup_question", paramName))
|
||||
}
|
||||
|
||||
try {
|
||||
if (!question) {
|
||||
task.consecutiveMistakeCount++
|
||||
task.recordToolError("ask_followup_question")
|
||||
task.didToolFailInCurrentTurn = true
|
||||
pushToolResult(await task.sayAndCreateMissingParamError("ask_followup_question", "question"))
|
||||
await recordMissingParamError("question")
|
||||
return
|
||||
}
|
||||
|
||||
if (!follow_up || !Array.isArray(follow_up)) {
|
||||
await recordMissingParamError("follow_up")
|
||||
return
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -13,7 +13,9 @@ describe("askFollowupQuestionTool", () => {
|
|||
mockCline = {
|
||||
ask: vi.fn().mockResolvedValue({ text: "Test response" }),
|
||||
say: vi.fn().mockResolvedValue(undefined),
|
||||
sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing parameter error"),
|
||||
consecutiveMistakeCount: 0,
|
||||
recordToolError: vi.fn(),
|
||||
}
|
||||
|
||||
mockPushToolResult = vi.fn((result) => {
|
||||
|
|
@ -109,6 +111,89 @@ describe("askFollowupQuestionTool", () => {
|
|||
)
|
||||
})
|
||||
|
||||
describe("parameter validation", () => {
|
||||
it("should handle missing follow_up parameter", 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: undefined 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()
|
||||
})
|
||||
|
||||
it("should handle null follow_up parameter", 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: null 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()
|
||||
})
|
||||
|
||||
it("should handle non-array follow_up parameter", 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: "not an array" 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("handlePartial with native protocol", () => {
|
||||
it("should only send question during partial streaming to avoid raw JSON display", async () => {
|
||||
const block: ToolUse<"ask_followup_question"> = {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue