Skip to content

Commit 6b64b25

Browse files
google-genai-botcopybara-github
authored andcommitted
fix: Fix ADK Runner race condition for sequential tool execution
PiperOrigin-RevId: 896524076
1 parent 7964e93 commit 6b64b25

3 files changed

Lines changed: 108 additions & 22 deletions

File tree

core/src/main/java/com/google/adk/flows/llmflows/BaseLlmFlow.java

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -461,14 +461,31 @@ public Flowable<Event> run(InvocationContext invocationContext) {
461461

462462
private Flowable<Event> run(
463463
Context spanContext, InvocationContext invocationContext, int stepsCompleted) {
464-
Flowable<Event> currentStepEvents = runOneStep(spanContext, invocationContext).cache();
464+
Flowable<Event> currentStepEvents = runOneStep(spanContext, invocationContext);
465+
466+
Flowable<Event> processedEvents =
467+
currentStepEvents
468+
.concatMap(
469+
event ->
470+
invocationContext
471+
.sessionService()
472+
.appendEvent(invocationContext.session(), event)
473+
.flatMap(
474+
registeredEvent ->
475+
invocationContext
476+
.pluginManager()
477+
.onEventCallback(invocationContext, registeredEvent)
478+
.defaultIfEmpty(registeredEvent))
479+
.toFlowable())
480+
.cache();
481+
465482
if (stepsCompleted + 1 >= maxSteps) {
466483
logger.debug("Ending flow execution because max steps reached.");
467-
return currentStepEvents;
484+
return processedEvents;
468485
}
469486

470-
return currentStepEvents.concatWith(
471-
currentStepEvents
487+
return processedEvents.concatWith(
488+
processedEvents
472489
.toList()
473490
.flatMapPublisher(
474491
eventList -> {

core/src/main/java/com/google/adk/runner/Runner.java

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -565,24 +565,29 @@ private Flowable<Event> runAgentWithUpdatedSession(
565565
.build());
566566

567567
// Agent execution
568-
Flowable<Event> agentEvents =
569-
contextWithUpdatedSession
570-
.agent()
571-
.runAsync(contextWithUpdatedSession)
572-
.concatMap(
573-
agentEvent ->
574-
this.sessionService
575-
.appendEvent(updatedSession, agentEvent)
576-
.flatMap(
577-
registeredEvent -> {
578-
// TODO: remove this hack after deprecating runAsync with Session.
579-
copySessionStates(updatedSession, initialContext.session());
580-
return contextWithUpdatedSession
581-
.pluginManager()
582-
.onEventCallback(contextWithUpdatedSession, registeredEvent)
583-
.defaultIfEmpty(registeredEvent);
584-
})
585-
.toFlowable());
568+
Flowable<Event> agentEvents;
569+
if (contextWithUpdatedSession.agent() instanceof LlmAgent) {
570+
agentEvents = contextWithUpdatedSession.agent().runAsync(contextWithUpdatedSession);
571+
} else {
572+
agentEvents =
573+
contextWithUpdatedSession
574+
.agent()
575+
.runAsync(contextWithUpdatedSession)
576+
.concatMap(
577+
agentEvent ->
578+
this.sessionService
579+
.appendEvent(updatedSession, agentEvent)
580+
.flatMap(
581+
registeredEvent -> {
582+
// TODO: remove this hack after deprecating runAsync with Session.
583+
copySessionStates(updatedSession, initialContext.session());
584+
return contextWithUpdatedSession
585+
.pluginManager()
586+
.onEventCallback(contextWithUpdatedSession, registeredEvent)
587+
.defaultIfEmpty(registeredEvent);
588+
})
589+
.toFlowable());
590+
}
586591

587592
// If beforeRunCallback returns content, emit it and skip agent
588593
Context capturedContext = Context.current();

core/src/test/java/com/google/adk/runner/RunnerTest.java

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
import com.google.adk.artifacts.BaseArtifactService;
4747
import com.google.adk.events.Event;
4848
import com.google.adk.flows.llmflows.Functions;
49+
import com.google.adk.models.LlmRequest;
4950
import com.google.adk.models.LlmResponse;
5051
import com.google.adk.plugins.BasePlugin;
5152
import com.google.adk.sessions.BaseSessionService;
@@ -1686,4 +1687,67 @@ public void runner_executesSaveArtifactFlow() {
16861687
// agent was run
16871688
assertThat(simplifyEvents(events.values())).containsExactly("test agent: from llm");
16881689
}
1690+
1691+
@Test
1692+
public void runAsync_ensuresSequentialConsistencyForTools() {
1693+
// Arrange
1694+
TestLlm testLlm =
1695+
createTestLlm(
1696+
createFunctionCallLlmResponse("call_1", "tool1", ImmutableMap.of("arg", "value1")),
1697+
createTextLlmResponse("Final response"));
1698+
1699+
LlmAgent agent =
1700+
createTestAgentBuilder(testLlm)
1701+
.tools(
1702+
ImmutableList.of(
1703+
FunctionTool.create(RaceConditionTools.class, "tool1"),
1704+
FunctionTool.create(RaceConditionTools.class, "tool2")))
1705+
.build();
1706+
1707+
Runner runner =
1708+
Runner.builder().app(App.builder().name("test").rootAgent(agent).build()).build();
1709+
Session session = runner.sessionService().createSession("test", "user").blockingGet();
1710+
1711+
// Act
1712+
var unused =
1713+
runner
1714+
.runAsync("user", session.id(), Content.fromParts(Part.fromText("start")))
1715+
.toList()
1716+
.blockingGet();
1717+
1718+
// Assert
1719+
List<LlmRequest> requests = testLlm.getRequests();
1720+
assertThat(requests).hasSize(2);
1721+
1722+
// Second request should contain the result of tool1
1723+
LlmRequest secondRequest = requests.get(1);
1724+
List<Content> history = secondRequest.contents();
1725+
1726+
boolean foundToolResponse = false;
1727+
for (Content content : history) {
1728+
for (Part part : content.parts().get()) {
1729+
if (part.functionResponse().isPresent()
1730+
&& part.functionResponse().get().name().isPresent()
1731+
&& "tool1".equals(part.functionResponse().get().name().get())) {
1732+
foundToolResponse = true;
1733+
assertThat(part.functionResponse().get().response().isPresent()).isTrue();
1734+
assertThat(part.functionResponse().get().response().get())
1735+
.isEqualTo(ImmutableMap.of("result", "result_value1"));
1736+
}
1737+
}
1738+
}
1739+
assertThat(foundToolResponse).isTrue();
1740+
}
1741+
1742+
public static class RaceConditionTools {
1743+
private RaceConditionTools() {}
1744+
1745+
public static ImmutableMap<String, Object> tool1(String arg) {
1746+
return ImmutableMap.of("result", "result_" + arg);
1747+
}
1748+
1749+
public static ImmutableMap<String, Object> tool2(String input) {
1750+
return ImmutableMap.of("status", "received_" + input);
1751+
}
1752+
}
16891753
}

0 commit comments

Comments
 (0)