Skip to content

Commit 7f89daa

Browse files
fix(tests): address roomote review - guard against undefined in pushToolResult and nest NativeToolCallParser tests
- Add assertion verifying pushToolResult argument does not contain 'undefined' - Move NativeToolCallParser integration tests into main describe block for better organization
1 parent 304f089 commit 7f89daa

1 file changed

Lines changed: 55 additions & 46 deletions

File tree

src/core/tools/__tests__/askFollowupQuestionTool.spec.ts

Lines changed: 55 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,9 @@ describe("AskFollowupQuestionTool", () => {
438438
await tool.execute(params, mockTask, mockCallbacks)
439439

440440
expect(mockTask.say).toHaveBeenCalledWith("user_feedback", "", [])
441+
// pushToolResult should receive normalized empty string, NOT "undefined"
442+
const toolResultArg = (mockCallbacks.pushToolResult as any).mock.calls[0][0]
443+
expect(toolResultArg).not.toContain("undefined")
441444
expect(mockCallbacks.pushToolResult).toHaveBeenCalledWith(
442445
formatResponse.toolResult("<user_message>\n\n</user_message>", []),
443446
)
@@ -447,55 +450,61 @@ describe("AskFollowupQuestionTool", () => {
447450
expect(askFollowupQuestionTool).toBeInstanceOf(AskFollowupQuestionTool)
448451
expect(askFollowupQuestionTool.name).toBe("ask_followup_question")
449452
})
450-
})
451453

452-
// ===== NativeToolCallParser integration tests for ask_followup_question =====
454+
// ===== NativeToolCallParser integration tests for ask_followup_question =====
453455

454-
describe("NativeToolCallParser.createPartialToolUse for ask_followup_question", () => {
455-
beforeEach(() => {
456-
NativeToolCallParser.clearAllStreamingToolCalls()
457-
NativeToolCallParser.clearRawChunkState()
458-
})
459-
460-
it("should build nativeArgs with question and follow_up during streaming", () => {
461-
NativeToolCallParser.startStreamingToolCall("call_123", "ask_followup_question")
456+
describe("NativeToolCallParser.createPartialToolUse for ask_followup_question", () => {
457+
beforeEach(() => {
458+
NativeToolCallParser.clearAllStreamingToolCalls()
459+
NativeToolCallParser.clearRawChunkState()
460+
})
462461

463-
const chunk1 = '{"question":"What would you like?","follow_up":[{"text":"Option 1","mode":"code"}'
464-
const result1 = NativeToolCallParser.processStreamingChunk("call_123", chunk1)
462+
it("should build nativeArgs with question and follow_up during streaming", () => {
463+
// Start a streaming tool call
464+
NativeToolCallParser.startStreamingToolCall("call_123", "ask_followup_question")
465+
466+
// Simulate streaming JSON chunks
467+
const chunk1 = '{"question":"What would you like?","follow_up":[{"text":"Option 1","mode":"code"}'
468+
const result1 = NativeToolCallParser.processStreamingChunk("call_123", chunk1)
469+
470+
expect(result1).not.toBeNull()
471+
expect(result1?.name).toBe("ask_followup_question")
472+
expect(result1?.params.question).toBe("What would you like?")
473+
expect(result1?.nativeArgs).toBeDefined()
474+
// Use type assertion to access the specific fields
475+
const nativeArgs = result1?.nativeArgs as {
476+
question: string
477+
follow_up?: Array<{ text: string; mode?: string }>
478+
}
479+
expect(nativeArgs?.question).toBe("What would you like?")
480+
// partial-json should parse the incomplete array
481+
expect(nativeArgs?.follow_up).toBeDefined()
482+
})
465483

466-
expect(result1).not.toBeNull()
467-
expect(result1?.name).toBe("ask_followup_question")
468-
expect(result1?.params.question).toBe("What would you like?")
469-
expect(result1?.nativeArgs).toBeDefined()
470-
const nativeArgs = result1?.nativeArgs as {
471-
question: string
472-
follow_up?: Array<{ text: string; mode?: string }>
473-
}
474-
expect(nativeArgs?.question).toBe("What would you like?")
475-
expect(nativeArgs?.follow_up).toBeDefined()
476-
})
477-
478-
it("should finalize with complete nativeArgs", () => {
479-
NativeToolCallParser.startStreamingToolCall("call_456", "ask_followup_question")
480-
481-
const completeJson =
482-
'{"question":"Choose an option","follow_up":[{"text":"Yes","mode":"code"},{"text":"No","mode":null}]}'
483-
NativeToolCallParser.processStreamingChunk("call_456", completeJson)
484-
485-
const result = NativeToolCallParser.finalizeStreamingToolCall("call_456")
486-
487-
expect(result).not.toBeNull()
488-
expect(result?.type).toBe("tool_use")
489-
expect(result?.name).toBe("ask_followup_question")
490-
expect(result?.partial).toBe(false)
491-
if (result?.type === "tool_use") {
492-
expect(result.nativeArgs).toEqual({
493-
question: "Choose an option",
494-
follow_up: [
495-
{ text: "Yes", mode: "code" },
496-
{ text: "No", mode: null },
497-
],
498-
})
499-
}
484+
it("should finalize with complete nativeArgs", () => {
485+
NativeToolCallParser.startStreamingToolCall("call_456", "ask_followup_question")
486+
487+
// Add complete JSON
488+
const completeJson =
489+
'{"question":"Choose an option","follow_up":[{"text":"Yes","mode":"code"},{"text":"No","mode":null}]}'
490+
NativeToolCallParser.processStreamingChunk("call_456", completeJson)
491+
492+
const result = NativeToolCallParser.finalizeStreamingToolCall("call_456")
493+
494+
expect(result).not.toBeNull()
495+
expect(result?.type).toBe("tool_use")
496+
expect(result?.name).toBe("ask_followup_question")
497+
expect(result?.partial).toBe(false)
498+
// Type guard: regular tools have type 'tool_use', MCP tools have type 'mcp_tool_use'
499+
if (result?.type === "tool_use") {
500+
expect(result.nativeArgs).toEqual({
501+
question: "Choose an option",
502+
follow_up: [
503+
{ text: "Yes", mode: "code" },
504+
{ text: "No", mode: null },
505+
],
506+
})
507+
}
508+
})
500509
})
501510
})

0 commit comments

Comments
 (0)