diff --git a/core/src/main/java/com/google/adk/flows/llmflows/RequestConfirmationLlmRequestProcessor.java b/core/src/main/java/com/google/adk/flows/llmflows/RequestConfirmationLlmRequestProcessor.java index 7f71898b9..acfaf33a4 100644 --- a/core/src/main/java/com/google/adk/flows/llmflows/RequestConfirmationLlmRequestProcessor.java +++ b/core/src/main/java/com/google/adk/flows/llmflows/RequestConfirmationLlmRequestProcessor.java @@ -82,12 +82,12 @@ public Single processRequest( ImmutableMap functionCallsById = functionCallsById(events, agentName); ImmutableSet confirmationRequestedIds = confirmationRequestedIds(events); - // A tool has been confirmed, but it might already have been executed by a subsequent processor - // or in a subsequent turn: such calls have a function response after the user confirmation + // A tool has been confirmed, but it might already have been executed in an earlier LLM step + // of this invocation: such calls have a function response after the user confirmation // event. This is applied before the resumability check rather than after, because - // findMostRecentConfirmations re-matches the same stale user event on every later LLM call, so - // a settled confirmation would otherwise be re-examined - and re-logged - for the rest of the - // session. + // findMostRecentConfirmations re-matches the same user event on every later LLM call of the + // invocation that answered it, so a settled confirmation would otherwise be re-examined - and + // re-logged - until the next user turn. // // Only responses this agent produced count. A peer event landing after the approval that // reuses the pending call's ID would otherwise convince this scan the tool had already run, @@ -167,11 +167,13 @@ public Single processRequest( private static Optional findMostRecentConfirmations( ImmutableList events) { - // Search backwards for the most recent user event that contains request confirmation - // function responses. + // Only the most recent user message can answer a pending confirmation. Scanning past a later + // user message without confirmation responses would re-apply a stale approval. User-authored + // compaction or state-only events carry no content and do not represent a new user turn. + // Skipping these contentless user events intentionally differs from ADK Python. for (int i = events.size() - 1; i >= 0; i--) { Event event = events.get(i); - if (!Objects.equals(event.author(), Role.USER) || event.functionResponses().isEmpty()) { + if (!Objects.equals(event.author(), Role.USER) || event.content().isEmpty()) { continue; } @@ -186,9 +188,9 @@ private static Optional findMostRecentConfirmations( .map(RequestConfirmationLlmRequestProcessor::maybeCreateToolConfirmationEntry) .flatMap(Optional::stream) .collect(toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)); - if (!confirmationsInEvent.isEmpty()) { - return Optional.of(new ConfirmationResult(confirmationsInEvent, i)); - } + return confirmationsInEvent.isEmpty() + ? Optional.empty() + : Optional.of(new ConfirmationResult(confirmationsInEvent, i)); } return Optional.empty(); } diff --git a/core/src/test/java/com/google/adk/flows/llmflows/RequestConfirmationLlmRequestProcessorTest.java b/core/src/test/java/com/google/adk/flows/llmflows/RequestConfirmationLlmRequestProcessorTest.java index 9ca8c767e..4bcdf676f 100644 --- a/core/src/test/java/com/google/adk/flows/llmflows/RequestConfirmationLlmRequestProcessorTest.java +++ b/core/src/test/java/com/google/adk/flows/llmflows/RequestConfirmationLlmRequestProcessorTest.java @@ -32,6 +32,7 @@ import com.google.adk.plugins.PluginManager; import com.google.adk.sessions.InMemorySessionService; import com.google.adk.sessions.Session; +import com.google.adk.summarizer.LlmEventSummarizer; import com.google.adk.testing.TestLlm; import com.google.adk.testing.TestUtils.EchoTool; import com.google.common.collect.ImmutableList; @@ -208,6 +209,147 @@ public void runAsync_noEvents_empty() { .isEmpty(); } + @Test + public void runAsync_compactionAfterApproval_callsOriginalFunction() { + LlmAgent agent = createAgentWithEchoTool(); + // Use the summarizer's actual event: its summary lives in actions, not in content. + Event compactionEvent = + new LlmEventSummarizer( + createTestLlm(createLlmResponse(Content.fromParts(Part.fromText("summary"))))) + .summarizeEvents(CONFIRMED_CALL_EVENTS) + .blockingGet(); + assertThat(compactionEvent).isNotNull(); + assertThat(compactionEvent.author()).isEqualTo("user"); + assertThat(compactionEvent.content()).isEmpty(); + Session session = + Session.builder("session_id") + .events( + ImmutableList.builder() + .addAll(CONFIRMED_CALL_EVENTS) + .add(compactionEvent) + .build()) + .build(); + + assertThat(resumedEvents(agent, session)).hasSize(1); + } + + @Test + public void runAsync_stateDeltaOnlyEventAfterApproval_callsOriginalFunction() { + LlmAgent agent = createAgentWithEchoTool(); + // Updating session state without a user message must not discard an unhandled approval. + Event stateDeltaEvent = + Event.builder() + .author("user") + .actions(EventActions.builder().stateDelta(ImmutableMap.of("resume_count", 1)).build()) + .build(); + Session session = + Session.builder("session_id") + .events( + ImmutableList.builder() + .addAll(CONFIRMED_CALL_EVENTS) + .add(stateDeltaEvent) + .build()) + .build(); + + assertThat(resumedEvents(agent, session)).hasSize(1); + } + + @Test + public void runAsync_userTextTurnThenContentlessEvent_doesNotCallOriginalFunction() { + LlmAgent agent = createAgentWithEchoTool(); + // Skipping an internal event must still stop at the intervening user message. + Session session = + Session.builder("session_id") + .events( + ImmutableList.builder() + .addAll(CONFIRMED_CALL_EVENTS) + .add( + Event.builder() + .author("user") + .content(Content.fromParts(Part.fromText("unrelated follow-up"))) + .build()) + .add(Event.builder().author("user").build()) + .build()) + .build(); + + assertThat(resumedEvents(agent, session)).isEmpty(); + } + + @Test + public void runAsync_emptyUserContentAfterApproval_doesNotCallOriginalFunction() { + LlmAgent agent = createAgentWithEchoTool(); + // Present but empty content is still a user message, not an internal contentless event. + Session session = + Session.builder("session_id") + .events( + ImmutableList.builder() + .addAll(CONFIRMED_CALL_EVENTS) + .add( + Event.builder() + .author("user") + .content(Content.builder().parts(ImmutableList.of()).build()) + .build()) + .build()) + .build(); + + assertThat(resumedEvents(agent, session)).isEmpty(); + } + + @Test + public void runAsync_userTextTurnAfterApproval_doesNotCallOriginalFunction() { + LlmAgent agent = createAgentWithEchoTool(); + // The approved call never produced a function response (its execution was aborted) and the + // user has since sent a plain text turn. ADK Python stops the scan at that turn, so the stale + // approval must not be applied again on this later LLM call. + Event laterUserTextEvent = + Event.builder() + .author("user") + .content(Content.fromParts(Part.fromText("unrelated follow-up question"))) + .build(); + Session session = + Session.builder("session_id") + .events( + ImmutableList.builder() + .addAll(CONFIRMED_CALL_EVENTS) + .add(laterUserTextEvent) + .build()) + .build(); + + assertThat(resumedEvents(agent, session)).isEmpty(); + } + + @Test + public void runAsync_laterUserTurnAnswersOtherFunctionCall_doesNotCallOriginalFunction() { + LlmAgent agent = createAgentWithEchoTool(); + // The most recent user event answers some other function call, not a confirmation request. + // Only that latest user turn is consulted, as in ADK Python, so the earlier approval is not + // re-applied either. + Event otherFunctionResponseEvent = + Event.builder() + .author("user") + .content( + Content.fromParts( + Part.builder() + .functionResponse( + FunctionResponse.builder() + .id("other_fc_id") + .name("other_tool") + .response(ImmutableMap.of("result", "done")) + .build()) + .build())) + .build(); + Session session = + Session.builder("session_id") + .events( + ImmutableList.builder() + .addAll(CONFIRMED_CALL_EVENTS) + .add(otherFunctionResponseEvent) + .build()) + .build(); + + assertThat(resumedEvents(agent, session)).isEmpty(); + } + @Test public void runAsync_noUserConfirmationEvent_empty() { LlmAgent agent = createAgentWithEchoTool();