Skip to content

Commit e5aba3a

Browse files
MiloszSobczykcopybara-github
authored andcommitted
fix(core): only resume tool confirmations for calls this agent emitted
PiperOrigin-RevId: 960227883
1 parent b75c916 commit e5aba3a

2 files changed

Lines changed: 419 additions & 28 deletions

File tree

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

Lines changed: 158 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import io.reactivex.rxjava3.core.Single;
4444
import java.util.Collection;
4545
import java.util.HashMap;
46+
import java.util.LinkedHashMap;
4647
import java.util.Map;
4748
import java.util.Objects;
4849
import java.util.Optional;
@@ -75,6 +76,28 @@ public Single<RequestProcessor.RequestProcessingResult> processRequest(
7576
int finalConfirmationEventIndex = confirmationResult.get().eventIndex();
7677
ImmutableMap<String, ToolConfirmation> requestConfirmationFunctionResponses =
7778
confirmationResult.get().responses();
79+
String agentName = invocationContext.agent().name();
80+
ImmutableMap<String, AuthoredFunctionCall> functionCallsById =
81+
functionCallsById(events, agentName);
82+
ImmutableSet<String> confirmationRequestedIds = confirmationRequestedIds(events);
83+
// A tool has been confirmed, but it might already have been executed by a subsequent processor
84+
// or in a subsequent turn: such calls have a function response after the user confirmation
85+
// event. This is applied before the resumability check rather than after, because
86+
// findMostRecentConfirmations re-matches the same stale user event on every later LLM call, so
87+
// a settled confirmation would otherwise be re-examined - and re-logged - for the rest of the
88+
// session.
89+
//
90+
// Only responses this agent produced count. A peer event landing after the approval that
91+
// reuses the pending call's ID would otherwise convince this scan the tool had already run,
92+
// silently dropping the approval - and it short-circuits before the resumability check, so
93+
// that would not even leave a log line.
94+
ImmutableSet<String> alreadyResumedIds =
95+
events.subList(finalConfirmationEventIndex + 1, events.size()).stream()
96+
.filter(event -> Objects.equals(event.author(), agentName))
97+
.flatMap(event -> event.functionResponses().stream())
98+
.map(FunctionResponse::id)
99+
.flatMap(Optional::stream)
100+
.collect(toImmutableSet());
78101

79102
// Search backwards from the event before confirmation for the corresponding
80103
// request_confirmation function calls emitted by the model.
@@ -83,6 +106,13 @@ public Single<RequestProcessor.RequestProcessingResult> processRequest(
83106
if (event.functionCalls().isEmpty()) {
84107
continue;
85108
}
109+
// Only this agent can ask this agent's user for confirmation. Function call parts also reach
110+
// the session from an A2A peer response - ResponseConverter turns one into a model-role event
111+
// authored by the local RemoteA2AAgent - and honouring a confirmation call from there would
112+
// let the peer choose which local tool runs.
113+
if (!Objects.equals(event.author(), agentName)) {
114+
continue;
115+
}
86116

87117
Map<String, ToolConfirmation> toolsToResumeWithConfirmation = new HashMap<>();
88118
Map<String, FunctionCall> toolsToResumeWithArgs = new HashMap<>();
@@ -95,6 +125,11 @@ public Single<RequestProcessor.RequestProcessingResult> processRequest(
95125
.forEach(
96126
fc ->
97127
getOriginalFunctionCall(fc)
128+
.filter(ofc -> !alreadyResumedIds.contains(ofc.id().get()))
129+
.filter(
130+
ofc ->
131+
isResumableFunctionCall(
132+
ofc, functionCallsById, confirmationRequestedIds, agentName))
98133
.ifPresent(
99134
ofc -> {
100135
toolsToResumeWithConfirmation.put(
@@ -103,23 +138,6 @@ public Single<RequestProcessor.RequestProcessingResult> processRequest(
103138
toolsToResumeWithArgs.put(ofc.id().get(), ofc);
104139
}));
105140

106-
if (toolsToResumeWithConfirmation.isEmpty()) {
107-
continue;
108-
}
109-
110-
// If a tool has been confirmed, it might have been executed by a subsequent
111-
// processor, or in a subsequent turn. We identify tools that have already been
112-
// executed by checking for function responses with matching IDs in events that
113-
// occurred *after* the user confirmation event.
114-
ImmutableSet<String> alreadyConfirmedIds =
115-
events.subList(finalConfirmationEventIndex + 1, events.size()).stream()
116-
.flatMap(e -> e.functionResponses().stream())
117-
.map(FunctionResponse::id)
118-
.flatMap(Optional::stream)
119-
.collect(toImmutableSet());
120-
toolsToResumeWithConfirmation.keySet().removeAll(alreadyConfirmedIds);
121-
toolsToResumeWithArgs.keySet().removeAll(alreadyConfirmedIds);
122-
123141
// If all confirmed tools in this event have already been processed, continue
124142
// searching in older events.
125143
if (toolsToResumeWithConfirmation.isEmpty()) {
@@ -173,6 +191,126 @@ private static Optional<ConfirmationResult> findMostRecentConfirmations(
173191
return Optional.empty();
174192
}
175193

194+
/**
195+
* Indexes the tool function calls in session history by ID, keeping the most recent one per ID.
196+
*
197+
* <p>Confirmation calls are excluded: a confirmation resumes a real tool call, never another
198+
* confirmation.
199+
*
200+
* <p>Collisions resolve last-wins, so a re-issue of an ID by {@code agentName} supersedes an
201+
* earlier one - except that a foreign author may never displace a call {@code agentName} emitted.
202+
* IDs are not globally unique and anyone can put an event in the session, so without that
203+
* precedence a peer could reuse the ID of a call this agent is waiting on, shadow it, and have
204+
* the author check in {@code isResumableFunctionCall} reject the <em>legitimate</em> confirmation
205+
* - turning that check into a way for a peer to veto any pending tool call.
206+
*/
207+
private static ImmutableMap<String, AuthoredFunctionCall> functionCallsById(
208+
ImmutableList<Event> events, String agentName) {
209+
Map<String, AuthoredFunctionCall> byId = new LinkedHashMap<>();
210+
for (Event event : events) {
211+
for (FunctionCall functionCall : event.functionCalls()) {
212+
if (functionCall.id().isEmpty()
213+
|| Objects.equals(
214+
functionCall.name().orElse(null), REQUEST_CONFIRMATION_FUNCTION_CALL_NAME)) {
215+
continue;
216+
}
217+
String id = functionCall.id().get();
218+
AuthoredFunctionCall existing = byId.get(id);
219+
if (existing == null
220+
|| Objects.equals(event.author(), agentName)
221+
|| !Objects.equals(existing.author(), agentName)) {
222+
byId.put(id, new AuthoredFunctionCall(event.author(), functionCall));
223+
}
224+
}
225+
}
226+
return ImmutableMap.copyOf(byId);
227+
}
228+
229+
/**
230+
* Collects the IDs of function calls that a tool actually asked the user to confirm.
231+
*
232+
* <p>Covers both ways a confirmation is requested: a tool calling {@link
233+
* com.google.adk.tools.ToolContext#requestConfirmation}, and a {@link
234+
* com.google.adk.tools.FunctionTool} created with {@code requireConfirmation}, which routes
235+
* through the same call. Accumulates over all events rather than keeping one event per ID:
236+
* re-executing a confirmed tool emits a second function response with the same ID and no
237+
* requested confirmations, which would otherwise shadow the original request.
238+
*/
239+
private static ImmutableSet<String> confirmationRequestedIds(ImmutableList<Event> events) {
240+
ImmutableSet.Builder<String> ids = ImmutableSet.builder();
241+
for (Event event : events) {
242+
Map<String, ToolConfirmation> requested = event.actions().requestedToolConfirmations();
243+
if (requested.isEmpty()) {
244+
continue;
245+
}
246+
for (FunctionResponse functionResponse : event.functionResponses()) {
247+
functionResponse.id().filter(requested::containsKey).ifPresent(ids::add);
248+
}
249+
}
250+
return ids.build();
251+
}
252+
253+
/**
254+
* Returns whether {@code originalFunctionCall} faithfully reproduces a tool call {@code
255+
* agentName} emitted and was genuinely awaiting confirmation.
256+
*
257+
* <p>The resumed call is read out of the {@code originalFunctionCall} argument of an {@code
258+
* adk_request_confirmation} call found in session history, and function call parts reach the
259+
* session from places other than the local model - notably an A2A peer response, which {@code
260+
* ResponseConverter} turns into a model-role event. Resuming such a call unchecked would let
261+
* whoever authored that event pick both the tool and its arguments, so only resume a call that
262+
* matches one this agent emitted, by ID, author, name and arguments, and that a tool actually
263+
* asked to have confirmed.
264+
*/
265+
private static boolean isResumableFunctionCall(
266+
FunctionCall originalFunctionCall,
267+
ImmutableMap<String, AuthoredFunctionCall> functionCallsById,
268+
ImmutableSet<String> confirmationRequestedIds,
269+
String agentName) {
270+
String id = originalFunctionCall.id().get();
271+
AuthoredFunctionCall emitted = functionCallsById.get(id);
272+
if (emitted == null) {
273+
logger.warn(
274+
"Ignoring tool confirmation for function call ID {}: no such function call in the session"
275+
+ " history.",
276+
id);
277+
return false;
278+
}
279+
if (!Objects.equals(emitted.author(), agentName)) {
280+
// Another agent emitted the call; leave it for that agent's own processor.
281+
logger.debug(
282+
"Skipping tool confirmation for function call ID {}: emitted by {}, not by {}.",
283+
id,
284+
emitted.author(),
285+
agentName);
286+
return false;
287+
}
288+
if (!Objects.equals(emitted.functionCall().name(), originalFunctionCall.name())) {
289+
logger.warn(
290+
"Ignoring tool confirmation for function call ID {}: tool name does not match the"
291+
+ " function call this agent emitted.",
292+
id);
293+
return false;
294+
}
295+
if (!Objects.equals(
296+
emitted.functionCall().args().orElse(ImmutableMap.of()),
297+
originalFunctionCall.args().orElse(ImmutableMap.of()))) {
298+
logger.warn(
299+
"Ignoring tool confirmation for function call ID {}: arguments do not match the function"
300+
+ " call this agent emitted.",
301+
id);
302+
return false;
303+
}
304+
if (!confirmationRequestedIds.contains(id)) {
305+
logger.warn(
306+
"Ignoring tool confirmation for function call ID {}: no tool requested confirmation for"
307+
+ " it.",
308+
id);
309+
return false;
310+
}
311+
return true;
312+
}
313+
176314
private Optional<FunctionCall> getOriginalFunctionCall(FunctionCall functionCall) {
177315
if (!functionCall.args().orElse(ImmutableMap.of()).containsKey(ORIGINAL_FUNCTION_CALL)) {
178316
return Optional.empty();
@@ -252,4 +390,7 @@ private static Optional<Map.Entry<String, ToolConfirmation>> maybeCreateToolConf
252390

253391
private record ConfirmationResult(
254392
ImmutableMap<String, ToolConfirmation> responses, int eventIndex) {}
393+
394+
/** A tool function call from session history, together with the author of its event. */
395+
private record AuthoredFunctionCall(String author, FunctionCall functionCall) {}
255396
}

0 commit comments

Comments
 (0)