Skip to content

Commit f4d6de4

Browse files
kvmiloscopybara-github
authored andcommitted
fix: restrict live transfer_to_agent to the agent, its sub-agents, parent, and peers
PiperOrigin-RevId: 1005160552
1 parent c781145 commit f4d6de4

3 files changed

Lines changed: 192 additions & 25 deletions

File tree

‎core/src/main/java/com/google/adk/agents/TransferRejectedException.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@
1818

1919
/**
2020
* Thrown when an {@link LlmAgent} calls {@code transfer_to_agent} with an agent it may not transfer
21-
* to: itself, an agent other than its sub-agents, parent, and peers, or one its {@code
22-
* disallowTransferToParent} or {@code disallowTransferToPeers} setting excludes.
21+
* to: one other than its sub-agents, parent, and peers; one its {@code disallowTransferToParent} or
22+
* {@code disallowTransferToPeers} setting excludes; or itself, outside a live run.
2323
*/
2424
public final class TransferRejectedException extends IllegalStateException {
2525

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

Lines changed: 39 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,8 @@ private Flowable<Event> runStepAndCheckContinue(
583583
* <p>Handles sending history and live requests to the LLM, receiving responses, processing them,
584584
* and managing agent transfers.
585585
*
586-
* @return A {@link Flowable} of {@link Event}s streamed in real-time.
586+
* @return A {@link Flowable} of {@link Event}s streamed in real-time, which fails with {@link
587+
* TransferRejectedException} if an agent in the run requests a transfer it may not make.
587588
*/
588589
@Override
589590
public Flowable<Event> runLive(InvocationContext invocationContext) {
@@ -703,28 +704,7 @@ public void onError(Throwable e) {
703704
llmResponse,
704705
callLlmContext);
705706
})
706-
.flatMap(
707-
event -> {
708-
Flowable<Event> events = Flowable.just(event);
709-
if (event.actions().transferToAgent().isPresent()) {
710-
BaseAgent rootAgent = invocationContext.agent().rootAgent();
711-
Optional<BaseAgent> nextAgent =
712-
rootAgent.findAgent(event.actions().transferToAgent().get());
713-
if (nextAgent.isEmpty()) {
714-
throw new IllegalStateException(
715-
"Agent not found: " + event.actions().transferToAgent().get());
716-
}
717-
Flowable<Event> nextAgentEvents =
718-
Flowable.defer(
719-
() -> {
720-
try (Scope scope = spanContext.makeCurrent()) {
721-
return nextAgent.get().runLive(invocationContext);
722-
}
723-
});
724-
events = Flowable.concat(events, nextAgentEvents);
725-
}
726-
return events;
727-
})
707+
.flatMap(event -> followLiveTransfer(event, invocationContext, spanContext))
728708
.doOnNext(
729709
event -> {
730710
ImmutableList<FunctionResponse> functionResponses =
@@ -752,6 +732,32 @@ public void onError(Throwable e) {
752732
}));
753733
}
754734

735+
/**
736+
* Emits {@code event}, followed by the target agent's live run when it carries a transfer. If the
737+
* agent may not make that transfer, fails with {@link TransferRejectedException} after {@code
738+
* event}, so the caller's {@code doOnNext} still sees the transfer and closes the connection.
739+
*/
740+
private static Flowable<Event> followLiveTransfer(
741+
Event event, InvocationContext context, Context spanContext) {
742+
Flowable<Event> self = Flowable.just(event);
743+
if (event.actions().transferToAgent().isEmpty()) {
744+
return self;
745+
}
746+
String agentToTransfer = event.actions().transferToAgent().get();
747+
LlmAgent agent = (LlmAgent) context.agent();
748+
Optional<BaseAgent> target = liveTransferTarget(agent, agentToTransfer);
749+
if (target.isEmpty()) {
750+
return self.concatWith(Flowable.error(transferRejected(agent, agentToTransfer)));
751+
}
752+
return self.concatWith(
753+
Flowable.defer(
754+
() -> {
755+
try (Scope scope = spanContext.makeCurrent()) {
756+
return target.get().runLive(context);
757+
}
758+
}));
759+
}
760+
755761
/**
756762
* Builds an {@link Event} from LLM response, request, and base event data.
757763
*
@@ -831,6 +837,16 @@ private static Optional<BaseAgent> declaredTransferTarget(LlmAgent from, String
831837
.findFirst();
832838
}
833839

840+
/**
841+
* Returns the agent a live transfer from {@code from} hands over to, or empty when the transfer
842+
* is not allowed. Unlike {@link #declaredTransferTarget}, returns {@code from} itself when {@code
843+
* name} names it, so a live run reopens it on a new connection; any other name must be a declared
844+
* target.
845+
*/
846+
private static Optional<BaseAgent> liveTransferTarget(LlmAgent from, String name) {
847+
return name.equals(from.name()) ? Optional.of(from) : declaredTransferTarget(from, name);
848+
}
849+
834850
/**
835851
* Returns the error for a transfer {@link #declaredTransferTarget} rejected. The message names
836852
* only {@code from}, since {@code name} is model output.

‎core/src/test/java/com/google/adk/flows/llmflows/AgentTransferTest.java‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
import io.reactivex.rxjava3.core.Flowable;
6161
import io.reactivex.rxjava3.core.Single;
6262
import io.reactivex.rxjava3.subscribers.TestSubscriber;
63+
import java.util.ArrayList;
6364
import java.util.List;
6465
import java.util.Map;
6566
import java.util.Optional;
@@ -242,6 +243,138 @@ public void runLive_transferToAgent_closesConnection() throws Exception {
242243
assertThat(closedConnectionsCount).isEqualTo(1);
243244
}
244245

246+
@Test
247+
public void runLive_transferToGrandchild_failsAndGrandchildNeverRuns() {
248+
TestLlm grandchildLlm = TestLlm.create(createTextLlmResponse("grandchild ran"));
249+
LlmAgent root = rootTransferringToGrandchild(grandchildLlm);
250+
251+
TransferRejectedException error =
252+
assertThrows(TransferRejectedException.class, () -> runLive(root));
253+
254+
assertThat(error)
255+
.hasMessageThat()
256+
.isEqualTo("Agent 'root' is not allowed to transfer to the requested agent.");
257+
assertThat(grandchildLlm.getRequests()).isEmpty();
258+
}
259+
260+
@Test
261+
public void runLive_transferToParent_withDisallowTransferToParent_fails() {
262+
// A second connection, so a wrongly re-run root ends the run instead of transferring again.
263+
TestLlm rootLlm =
264+
createTestLlm(
265+
Flowable.just(transferTo("sub")),
266+
Flowable.just(createTextLlmResponse("root ran again")));
267+
LlmAgent root = rootWithSubDisallowingParent(rootLlm);
268+
269+
TransferRejectedException error =
270+
assertThrows(TransferRejectedException.class, () -> runLive(root));
271+
272+
assertThat(error)
273+
.hasMessageThat()
274+
.isEqualTo("Agent 'sub' is not allowed to transfer to the requested agent.");
275+
assertThat(rootLlm.getRequests()).hasSize(1);
276+
}
277+
278+
@Test
279+
public void runLive_transferToPeer_withDisallowTransferToPeers_failsAndPeerNeverRuns() {
280+
TestLlm peerLlm = TestLlm.create(ImmutableList.of());
281+
LlmAgent root = rootWithSubDisallowingPeers(TestLlm.create(transferTo("peer")), peerLlm);
282+
283+
TransferRejectedException error =
284+
assertThrows(TransferRejectedException.class, () -> runLive(root));
285+
286+
assertThat(error)
287+
.hasMessageThat()
288+
.isEqualTo("Agent 'sub' is not allowed to transfer to the requested agent.");
289+
assertThat(error.agentName()).isEqualTo("sub");
290+
assertThat(peerLlm.getRequests()).isEmpty();
291+
}
292+
293+
// The model chose the name, so the error names only the calling agent.
294+
@Test
295+
public void runLive_transferToUnknownAgent_failsAsNotInTreeAfterClosingTheConnection() {
296+
TestLlm rootLlm = TestLlm.create(transferTo("missing"));
297+
LlmAgent root = root(rootLlm, textAgent("sub"));
298+
List<Event> events = new ArrayList<>();
299+
300+
TransferRejectedException error =
301+
assertThrows(TransferRejectedException.class, () -> runLive(root, events));
302+
303+
assertThat(error)
304+
.hasMessageThat()
305+
.isEqualTo("Agent 'root' requested a transfer to an agent that is not in the agent tree.");
306+
assertThat(simplifyEvents(events))
307+
.containsExactly(
308+
"root: FunctionCall(name=transfer_to_agent, args={agent_name=missing})",
309+
"root: FunctionResponse(name=transfer_to_agent, response={})")
310+
.inOrder();
311+
assertThat(rootLlm.getLiveRequestHistory().stream().filter(LiveRequest::shouldClose).count())
312+
.isEqualTo(1);
313+
}
314+
315+
@Test
316+
public void runLive_transferToDuplicateName_runsTheDeclaredTarget() {
317+
TestLlm declaredLlm = TestLlm.create(createTextLlmResponse("declared ran"));
318+
TestLlm namesakeLlm = TestLlm.create(createTextLlmResponse("namesake ran"));
319+
320+
ImmutableList<Event> events = runLive(rootWithNamesakeTargets(declaredLlm, namesakeLlm));
321+
322+
assertThat(simplifyEvents(events)).contains("shared: declared ran");
323+
assertThat(declaredLlm.getRequests()).hasSize(1);
324+
assertThat(namesakeLlm.getRequests()).isEmpty();
325+
}
326+
327+
// Peers and the parent are declared targets too, not only sub-agents.
328+
@Test
329+
public void runLive_transferToPeerThenParent_runsBoth() {
330+
TestLlm rootLlm =
331+
createTestLlm(
332+
Flowable.just(transferTo("sub")), Flowable.just(createTextLlmResponse("root again")));
333+
TestLlm peerLlm = TestLlm.create(transferTo("root"));
334+
LlmAgent sub = createTestAgentBuilder(TestLlm.create(transferTo("peer"))).name("sub").build();
335+
LlmAgent peer = createTestAgentBuilder(peerLlm).name("peer").build();
336+
337+
ImmutableList<Event> events = runLive(root(rootLlm, sub, peer));
338+
339+
assertThat(simplifyEvents(events)).contains("root: root again");
340+
assertThat(peerLlm.getRequests()).hasSize(1);
341+
assertThat(rootLlm.getRequests()).hasSize(2);
342+
}
343+
344+
// runAsync rejects a transfer to self; a live run reopens the agent on a new connection.
345+
@Test
346+
public void runLive_transferToSelf_reopensTheAgent() {
347+
TestLlm rootLlm =
348+
createTestLlm(
349+
Flowable.just(transferTo("root")),
350+
Flowable.just(createTextLlmResponse("root reopened")));
351+
352+
ImmutableList<Event> events = runLive(root(rootLlm, textAgent("sub")));
353+
354+
assertThat(simplifyEvents(events))
355+
.containsExactly(
356+
"root: FunctionCall(name=transfer_to_agent, args={agent_name=root})",
357+
"root: FunctionResponse(name=transfer_to_agent, response={})",
358+
"root: root reopened")
359+
.inOrder();
360+
assertThat(rootLlm.getRequests()).hasSize(2);
361+
}
362+
363+
@Test
364+
public void runLive_transferToSelf_reopensTheCallerNotANamesake() {
365+
TestLlm callerLlm =
366+
createTestLlm(
367+
Flowable.just(transferTo("shared")),
368+
Flowable.just(createTextLlmResponse("caller reopened")));
369+
TestLlm namesakeLlm = TestLlm.create(createTextLlmResponse("namesake ran"));
370+
371+
ImmutableList<Event> events = runLive(rootWithNamesakeTargets(callerLlm, namesakeLlm));
372+
373+
assertThat(simplifyEvents(events)).contains("shared: caller reopened");
374+
assertThat(callerLlm.getRequests()).hasSize(2);
375+
assertThat(namesakeLlm.getRequests()).isEmpty();
376+
}
377+
245378
@Test
246379
public void testAutoToAuto() {
247380
Content transferCallContent = Content.fromParts(createTransferCallPart("sub_agent_1"));
@@ -953,4 +1086,22 @@ private List<Event> runRunner(Runner runner, InvocationContext invocationContext
9531086
.toList()
9541087
.blockingGet();
9551088
}
1089+
1090+
private static ImmutableList<Event> runLive(LlmAgent root) {
1091+
List<Event> events = new ArrayList<>();
1092+
runLive(root, events);
1093+
return ImmutableList.copyOf(events);
1094+
}
1095+
1096+
/** Runs {@code root} live after one user message, adding each event to {@code events}. */
1097+
private static void runLive(LlmAgent root, List<Event> events) {
1098+
Runner runner = new InMemoryRunner(root, "test");
1099+
LiveRequestQueue liveRequestQueue = new LiveRequestQueue();
1100+
liveRequestQueue.content(Content.fromParts(Part.fromText("hi")));
1101+
runner
1102+
.runLive(newSession(runner), liveRequestQueue, RunConfig.builder().build())
1103+
.doOnNext(events::add)
1104+
.ignoreElements()
1105+
.blockingAwait();
1106+
}
9561107
}

0 commit comments

Comments
 (0)