From b087891ffa4b19fcfd345316d706ace1c4c4e1e9 Mon Sep 17 00:00:00 2001 From: Gustaf Halvardsson Date: Tue, 28 Apr 2026 10:17:36 +0200 Subject: [PATCH] fix: yield function responses before tool confirmations A consumer that pauses on receiving an adk_request_confirmation event (typically to await human approval) would never see the merged function-response event for tools that already executed in the same step. Yield completed responses first so they are persisted before the human-confirmation pause. Fixes google/adk-go#759 --- internal/llminternal/base_flow.go | 11 ++-- tool/functiontool/function_test.go | 90 ++++++++++++++++++++++++------ tool/mcptoolset/set_test.go | 42 +++++++------- 3 files changed, 100 insertions(+), 43 deletions(-) diff --git a/internal/llminternal/base_flow.go b/internal/llminternal/base_flow.go index 1a5dfa021..5be32df23 100644 --- a/internal/llminternal/base_flow.go +++ b/internal/llminternal/base_flow.go @@ -204,16 +204,19 @@ func (f *Flow) runOneStep(ctx agent.InvocationContext) iter.Seq2[*session.Event, } toolConfirmationEvent := generateRequestConfirmationEvent(ctx, modelResponseEvent, ev) + + // Yield function responses before confirmation requests so consumers that + // pause for user approval still persist completed tool results. + if !yield(ev, nil) { + return + } + if toolConfirmationEvent != nil { if !yield(toolConfirmationEvent, nil) { return } } - if !yield(ev, nil) { - return - } - // If the model response is structured, yield it as a final model response event. outputSchemaResponse, err := retrieveStructuredModelResponse(ev) if err != nil { diff --git a/tool/functiontool/function_test.go b/tool/functiontool/function_test.go index 570b726ab..f7cf24588 100644 --- a/tool/functiontool/function_test.go +++ b/tool/functiontool/function_test.go @@ -584,6 +584,9 @@ func TestToolConfirmation(t *testing.T) { args: map[string]any{"Num": 1}, want: []*genai.Content{ genai.NewContentFromFunctionCall("test_tool", map[string]any{"Num": 1}, "model"), + genai.NewContentFromFunctionResponse("test_tool", map[string]any{ + "error": errors.New("error tool \"test_tool\" requires confirmation, please approve or reject"), + }, "user"), genai.NewContentFromFunctionCall(toolconfirmation.FunctionCallName, map[string]any{ "originalFunctionCall": &genai.FunctionCall{ Args: map[string]any{"Num": 1}, @@ -593,9 +596,6 @@ func TestToolConfirmation(t *testing.T) { Hint: "Please approve or reject the tool call test_tool() by responding with a FunctionResponse with an expected ToolConfirmation payload.", }, }, "model"), - genai.NewContentFromFunctionResponse("test_tool", map[string]any{ - "error": errors.New("error tool \"test_tool\" requires confirmation, please approve or reject"), - }, "user"), }, }, { @@ -608,6 +608,9 @@ func TestToolConfirmation(t *testing.T) { confirmFunctionResponse: &genai.FunctionResponse{Name: toolconfirmation.FunctionCallName, Response: map[string]any{"confirmed": true}}, want: []*genai.Content{ genai.NewContentFromFunctionCall("test_tool", map[string]any{"Num": 1}, "model"), + genai.NewContentFromFunctionResponse("test_tool", map[string]any{ + "error": errors.New("error tool \"test_tool\" requires confirmation, please approve or reject"), + }, "user"), genai.NewContentFromFunctionCall(toolconfirmation.FunctionCallName, map[string]any{ "originalFunctionCall": &genai.FunctionCall{ Args: map[string]any{"Num": 1}, @@ -617,9 +620,6 @@ func TestToolConfirmation(t *testing.T) { Hint: "Please approve or reject the tool call test_tool() by responding with a FunctionResponse with an expected ToolConfirmation payload.", }, }, "model"), - genai.NewContentFromFunctionResponse("test_tool", map[string]any{ - "error": errors.New("error tool \"test_tool\" requires confirmation, please approve or reject"), - }, "user"), genai.NewContentFromFunctionResponse("test_tool", map[string]any{"result": "ok"}, "user"), }, }, @@ -633,6 +633,9 @@ func TestToolConfirmation(t *testing.T) { confirmFunctionResponse: &genai.FunctionResponse{Name: toolconfirmation.FunctionCallName, Response: map[string]any{"confirmed": false}}, want: []*genai.Content{ genai.NewContentFromFunctionCall("test_tool", map[string]any{"Num": 1}, "model"), + genai.NewContentFromFunctionResponse("test_tool", map[string]any{ + "error": errors.New("error tool \"test_tool\" requires confirmation, please approve or reject"), + }, "user"), genai.NewContentFromFunctionCall(toolconfirmation.FunctionCallName, map[string]any{ "originalFunctionCall": &genai.FunctionCall{ Args: map[string]any{"Num": 1}, @@ -642,9 +645,6 @@ func TestToolConfirmation(t *testing.T) { Hint: "Please approve or reject the tool call test_tool() by responding with a FunctionResponse with an expected ToolConfirmation payload.", }, }, "model"), - genai.NewContentFromFunctionResponse("test_tool", map[string]any{ - "error": errors.New("error tool \"test_tool\" requires confirmation, please approve or reject"), - }, "user"), genai.NewContentFromFunctionResponse("test_tool", map[string]any{ "error": errors.New("error tool \"test_tool\" call is rejected"), }, "user"), @@ -675,6 +675,9 @@ func TestToolConfirmation(t *testing.T) { args: map[string]any{"Num": 4}, want: []*genai.Content{ genai.NewContentFromFunctionCall("test_tool", map[string]any{"Num": 4}, "model"), + genai.NewContentFromFunctionResponse("test_tool", map[string]any{ + "error": errors.New("error tool \"test_tool\" requires confirmation, please approve or reject"), + }, "user"), genai.NewContentFromFunctionCall(toolconfirmation.FunctionCallName, map[string]any{ "originalFunctionCall": &genai.FunctionCall{ Args: map[string]any{"Num": 4}, @@ -684,9 +687,6 @@ func TestToolConfirmation(t *testing.T) { Hint: "Please approve or reject the tool call test_tool() by responding with a FunctionResponse with an expected ToolConfirmation payload.", }, }, "model"), - genai.NewContentFromFunctionResponse("test_tool", map[string]any{ - "error": errors.New("error tool \"test_tool\" requires confirmation, please approve or reject"), - }, "user"), }, }, { @@ -701,6 +701,9 @@ func TestToolConfirmation(t *testing.T) { confirmFunctionResponse: &genai.FunctionResponse{Name: toolconfirmation.FunctionCallName, Response: map[string]any{"confirmed": true}}, want: []*genai.Content{ genai.NewContentFromFunctionCall("test_tool", map[string]any{"Num": 4}, "model"), + genai.NewContentFromFunctionResponse("test_tool", map[string]any{ + "error": errors.New("error tool \"test_tool\" requires confirmation, please approve or reject"), + }, "user"), genai.NewContentFromFunctionCall(toolconfirmation.FunctionCallName, map[string]any{ "originalFunctionCall": &genai.FunctionCall{ Args: map[string]any{"Num": 4}, @@ -710,9 +713,6 @@ func TestToolConfirmation(t *testing.T) { Hint: "Please approve or reject the tool call test_tool() by responding with a FunctionResponse with an expected ToolConfirmation payload.", }, }, "model"), - genai.NewContentFromFunctionResponse("test_tool", map[string]any{ - "error": errors.New("error tool \"test_tool\" requires confirmation, please approve or reject"), - }, "user"), genai.NewContentFromFunctionResponse("test_tool", map[string]any{"result": "ok"}, "user"), }, }, @@ -728,6 +728,9 @@ func TestToolConfirmation(t *testing.T) { confirmFunctionResponse: &genai.FunctionResponse{Name: toolconfirmation.FunctionCallName, Response: map[string]any{"confirmed": false}}, want: []*genai.Content{ genai.NewContentFromFunctionCall("test_tool", map[string]any{"Num": 4}, "model"), + genai.NewContentFromFunctionResponse("test_tool", map[string]any{ + "error": errors.New("error tool \"test_tool\" requires confirmation, please approve or reject"), + }, "user"), genai.NewContentFromFunctionCall(toolconfirmation.FunctionCallName, map[string]any{ "originalFunctionCall": &genai.FunctionCall{ Args: map[string]any{"Num": 4}, @@ -737,9 +740,6 @@ func TestToolConfirmation(t *testing.T) { Hint: "Please approve or reject the tool call test_tool() by responding with a FunctionResponse with an expected ToolConfirmation payload.", }, }, "model"), - genai.NewContentFromFunctionResponse("test_tool", map[string]any{ - "error": errors.New("error tool \"test_tool\" requires confirmation, please approve or reject"), - }, "user"), genai.NewContentFromFunctionResponse("test_tool", map[string]any{ "error": errors.New("error tool \"test_tool\" call is rejected"), }, "user"), @@ -865,6 +865,60 @@ func TestToolConfirmation(t *testing.T) { } } +func TestToolConfirmationYieldsFunctionResponseBeforeConfirmation(t *testing.T) { + mockModel := &testutil.MockModel{ + Responses: []*genai.Content{ + genai.NewContentFromFunctionCall("test_tool", map[string]any{"Num": 1}, genai.RoleModel), + }, + } + + myTool, err := functiontool.New(functiontool.Config{ + Name: "test_tool", + RequireConfirmation: true, + }, okFunc) + if err != nil { + t.Fatalf("Failed to create tool: %v", err) + } + + a, err := llmagent.New(llmagent.Config{ + Name: "simple agent", + Model: mockModel, + Tools: []tool.Tool{myTool}, + }) + if err != nil { + t.Fatalf("failed to create llm agent: %v", err) + } + + runner := testutil.NewTestAgentRunner(t, a) + sawFunctionResponse := false + sawConfirmation := false + + for got, err := range runner.Run(t, "id", "message") { + // The mock model emits a sentinel error once exhausted; treat any + // error as end-of-stream and rely on the post-loop assertions. + if err != nil { + break + } + + for _, part := range got.Content.Parts { + if part.FunctionResponse != nil && part.FunctionResponse.Name == "test_tool" { + sawFunctionResponse = true + } + if part.FunctionCall != nil && part.FunctionCall.Name == toolconfirmation.FunctionCallName { + sawConfirmation = true + if !sawFunctionResponse { + t.Fatal("confirmation was yielded before the tool function response") + } + return + } + } + } + + if !sawConfirmation { + t.Fatal("expected confirmation event") + } +} + // Mock types for TArgs and TResults type TestArgs struct { Name string diff --git a/tool/mcptoolset/set_test.go b/tool/mcptoolset/set_test.go index 1fe3cb512..bcd9e38b5 100644 --- a/tool/mcptoolset/set_test.go +++ b/tool/mcptoolset/set_test.go @@ -415,6 +415,9 @@ func TestMCPToolSetConfirmation(t *testing.T) { city: "Lisbon", want: []*genai.Content{ genai.NewContentFromFunctionCall(toolName, map[string]any{"city": "Lisbon"}, "model"), + genai.NewContentFromFunctionResponse(toolName, map[string]any{ + "error": errors.New("error tool \"get_weather\" requires confirmation, please approve or reject"), + }, "user"), genai.NewContentFromFunctionCall(toolconfirmation.FunctionCallName, map[string]any{ "originalFunctionCall": &genai.FunctionCall{ Args: map[string]any{"city": "Lisbon"}, @@ -424,9 +427,6 @@ func TestMCPToolSetConfirmation(t *testing.T) { Hint: "Please approve or reject the tool call get_weather() by responding with a FunctionResponse with an expected ToolConfirmation payload.", }, }, "model"), - genai.NewContentFromFunctionResponse(toolName, map[string]any{ - "error": errors.New("error tool \"get_weather\" requires confirmation, please approve or reject"), - }, "user"), }, }, { @@ -438,6 +438,9 @@ func TestMCPToolSetConfirmation(t *testing.T) { confirmFunctionResponse: &genai.FunctionResponse{Name: toolconfirmation.FunctionCallName, Response: map[string]any{"confirmed": true}}, want: []*genai.Content{ genai.NewContentFromFunctionCall(toolName, map[string]any{"city": "Lisbon"}, "model"), + genai.NewContentFromFunctionResponse(toolName, map[string]any{ + "error": errors.New("error tool \"get_weather\" requires confirmation, please approve or reject"), + }, "user"), genai.NewContentFromFunctionCall(toolconfirmation.FunctionCallName, map[string]any{ "originalFunctionCall": &genai.FunctionCall{ Args: map[string]any{"city": "Lisbon"}, @@ -447,9 +450,6 @@ func TestMCPToolSetConfirmation(t *testing.T) { Hint: "Please approve or reject the tool call get_weather() by responding with a FunctionResponse with an expected ToolConfirmation payload.", }, }, "model"), - genai.NewContentFromFunctionResponse(toolName, map[string]any{ - "error": errors.New("error tool \"get_weather\" requires confirmation, please approve or reject"), - }, "user"), genai.NewContentFromFunctionResponse(toolName, map[string]any{ "output": map[string]any{"weather_summary": string(`Today in "Lisbon" is sunny`)}, }, "user"), @@ -465,6 +465,9 @@ func TestMCPToolSetConfirmation(t *testing.T) { confirmFunctionResponse: &genai.FunctionResponse{Name: toolconfirmation.FunctionCallName, Response: map[string]any{"confirmed": false}}, want: []*genai.Content{ genai.NewContentFromFunctionCall(toolName, map[string]any{"city": "Lisbon"}, "model"), + genai.NewContentFromFunctionResponse(toolName, map[string]any{ + "error": errors.New("error tool \"get_weather\" requires confirmation, please approve or reject"), + }, "user"), genai.NewContentFromFunctionCall(toolconfirmation.FunctionCallName, map[string]any{ "originalFunctionCall": &genai.FunctionCall{ Args: map[string]any{"city": "Lisbon"}, @@ -474,9 +477,6 @@ func TestMCPToolSetConfirmation(t *testing.T) { Hint: "Please approve or reject the tool call get_weather() by responding with a FunctionResponse with an expected ToolConfirmation payload.", }, }, "model"), - genai.NewContentFromFunctionResponse(toolName, map[string]any{ - "error": errors.New("error tool \"get_weather\" requires confirmation, please approve or reject"), - }, "user"), genai.NewContentFromFunctionResponse(toolName, map[string]any{ "error": errors.New("error tool \"get_weather\" call is rejected"), }, "user"), @@ -523,6 +523,9 @@ func TestMCPToolSetConfirmation(t *testing.T) { city: "Lisbon", want: []*genai.Content{ genai.NewContentFromFunctionCall(toolName, map[string]any{"city": "Lisbon"}, "model"), + genai.NewContentFromFunctionResponse(toolName, map[string]any{ + "error": errors.New("error tool \"get_weather\" requires confirmation, please approve or reject"), + }, "user"), genai.NewContentFromFunctionCall(toolconfirmation.FunctionCallName, map[string]any{ "originalFunctionCall": &genai.FunctionCall{ Args: map[string]any{"city": "Lisbon"}, @@ -532,9 +535,6 @@ func TestMCPToolSetConfirmation(t *testing.T) { Hint: "Please approve or reject the tool call get_weather() by responding with a FunctionResponse with an expected ToolConfirmation payload.", }, }, "model"), - genai.NewContentFromFunctionResponse(toolName, map[string]any{ - "error": errors.New("error tool \"get_weather\" requires confirmation, please approve or reject"), - }, "user"), }, }, { @@ -545,6 +545,9 @@ func TestMCPToolSetConfirmation(t *testing.T) { city: "Lisbon", want: []*genai.Content{ genai.NewContentFromFunctionCall(toolName, map[string]any{"city": "Lisbon"}, "model"), + genai.NewContentFromFunctionResponse(toolName, map[string]any{ + "error": errors.New("error tool \"get_weather\" requires confirmation, please approve or reject"), + }, "user"), genai.NewContentFromFunctionCall(toolconfirmation.FunctionCallName, map[string]any{ "originalFunctionCall": &genai.FunctionCall{ Args: map[string]any{"city": "Lisbon"}, @@ -554,9 +557,6 @@ func TestMCPToolSetConfirmation(t *testing.T) { Hint: "Please approve or reject the tool call get_weather() by responding with a FunctionResponse with an expected ToolConfirmation payload.", }, }, "model"), - genai.NewContentFromFunctionResponse(toolName, map[string]any{ - "error": errors.New("error tool \"get_weather\" requires confirmation, please approve or reject"), - }, "user"), }, }, { @@ -568,6 +568,9 @@ func TestMCPToolSetConfirmation(t *testing.T) { confirmFunctionResponse: &genai.FunctionResponse{Name: toolconfirmation.FunctionCallName, Response: map[string]any{"confirmed": true}}, want: []*genai.Content{ genai.NewContentFromFunctionCall(toolName, map[string]any{"city": "Lisbon"}, "model"), + genai.NewContentFromFunctionResponse(toolName, map[string]any{ + "error": errors.New("error tool \"get_weather\" requires confirmation, please approve or reject"), + }, "user"), genai.NewContentFromFunctionCall(toolconfirmation.FunctionCallName, map[string]any{ "originalFunctionCall": &genai.FunctionCall{ Args: map[string]any{"city": "Lisbon"}, @@ -577,9 +580,6 @@ func TestMCPToolSetConfirmation(t *testing.T) { Hint: "Please approve or reject the tool call get_weather() by responding with a FunctionResponse with an expected ToolConfirmation payload.", }, }, "model"), - genai.NewContentFromFunctionResponse(toolName, map[string]any{ - "error": errors.New("error tool \"get_weather\" requires confirmation, please approve or reject"), - }, "user"), genai.NewContentFromFunctionResponse(toolName, map[string]any{ "output": map[string]any{"weather_summary": string(`Today in "Lisbon" is sunny`)}, }, "user"), @@ -595,6 +595,9 @@ func TestMCPToolSetConfirmation(t *testing.T) { confirmFunctionResponse: &genai.FunctionResponse{Name: toolconfirmation.FunctionCallName, Response: map[string]any{"confirmed": false}}, want: []*genai.Content{ genai.NewContentFromFunctionCall(toolName, map[string]any{"city": "Lisbon"}, "model"), + genai.NewContentFromFunctionResponse(toolName, map[string]any{ + "error": errors.New("error tool \"get_weather\" requires confirmation, please approve or reject"), + }, "user"), genai.NewContentFromFunctionCall(toolconfirmation.FunctionCallName, map[string]any{ "originalFunctionCall": &genai.FunctionCall{ Args: map[string]any{"city": "Lisbon"}, @@ -604,9 +607,6 @@ func TestMCPToolSetConfirmation(t *testing.T) { Hint: "Please approve or reject the tool call get_weather() by responding with a FunctionResponse with an expected ToolConfirmation payload.", }, }, "model"), - genai.NewContentFromFunctionResponse(toolName, map[string]any{ - "error": errors.New("error tool \"get_weather\" requires confirmation, please approve or reject"), - }, "user"), genai.NewContentFromFunctionResponse(toolName, map[string]any{ "error": errors.New("error tool \"get_weather\" call is rejected"), }, "user"),