Commit 883ff98
fix(flows): terminate invocation at tool-level EUC
Merge #5638
### Link to Issue or Description of Change
**1. Link to an existing issue (if applicable):**
- Closes: #5637
This change adds `invocation_context.end_invocation = True` after the auth event yield in `_postprocess_handle_function_calls_async`, mirroring the existing termination signal in `_resolve_toolset_auth`. Tool-level auth now terminates symmetrically with toolset-level auth at the EUC, instead of continuing for one more LLM call.
### Testing Plan
**Unit Tests:**
- [x] I have added or updated unit tests for my change.
- [x] All unit tests pass locally.
Three existing tests in `test_functions_request_euc.py` had assertions tied to the trailing post-EUC LLM call:
- `test_function_request_euc`: adds `assert len(mock_model.requests) == 1` to anchor the new termination behavior.
- `test_function_get_auth_response`: `events[-3]` → `events[-2]` for the auth event lookup, since the auth event is now second-to-last.
- `test_function_get_auth_response_partial`: same `events[-3]` → `events[-2]` change, plus the two `len(mock_model.requests)` assertions drop by 1 (3 → 2 and 4 → 3).
```
$ pytest tests/unittests/flows/llm_flows/test_functions_request_euc.py
======================== 3 passed, 17 warnings in 1.31s ========================
$ pytest tests/unittests/
=============== 5695 passed, 2308 warnings in 122.89s (0:02:02) ================
```
**Manual End-to-End (E2E) Tests:**
A self-contained Runner-based reproduction is at https://github.com/doughayden/adk-issue-examples/tree/main/04-tool_level_auth_continuation. The agent definition (`agent.py`) wires up an `OpenAPIToolset` against a local OAuth2 test server. `main.py` constructs an `InMemoryRunner`, applies the workaround for #5327 (`get_auth_config = lambda: None`) at runtime to land on the tool-level auth path, and sends a tool-triggering prompt. The `--apply-fix` flag monkey-patches the proposed fix to demonstrate the resolution end-to-end.
Without the fix:
```
👤 User: What's the weather in San Francisco?
🌤️ Weather Assistant event stream:
[function_call] get_weather by WeatherAssistant
[auth_event] adk_request_credential by WeatherAssistant
[function_response] get_weather by WeatherAssistant
[post_euc_text] WeatherAssistant: "I'm sorry, I cannot retrieve the weather for San Francisco at the moment. It ..."
Event counts:
function_calls: 1
auth_events: 1
function_responses: 1
text_events: 1
post_euc_text_events: 1
✅ Bug reproduced: 1 text event(s) after the EUC (agent loop continued past adk_request_credential).
```
With the fix:
```
👤 User: What's the weather in San Francisco?
🌤️ Weather Assistant event stream:
[function_call] get_weather by WeatherAssistant
[auth_event] adk_request_credential by WeatherAssistant
[function_response] get_weather by WeatherAssistant
Event counts:
function_calls: 1
auth_events: 1
function_responses: 1
text_events: 0
post_euc_text_events: 0
✅ Fix verified: no LLM events after the EUC.
```
### Checklist
- [x] I have read the [CONTRIBUTING.md](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) document.
- [x] I have performed a self-review of my own code.
- [x] I have commented my code, particularly in hard-to-understand areas.
- [x] I have added tests that prove my fix is effective or that my feature works.
- [x] New and existing unit tests pass locally with my changes.
- [x] I have manually tested my changes end-to-end.
- [ ] Any dependent changes have been merged and published in downstream modules.
### Additional context
**Alternative considered:**
A reorder of the yields (yield `auth_event` last so `last_event.is_final_response()` returns True) would also fix the loop termination in a single iteration without needing the flag. I went with `end_invocation = True` to preserve the observable event order and to match the existing pattern in `_resolve_toolset_auth`. Happy to switch if maintainers prefer the reorder.
**Related:**
The same yield site at lines 1126-1130 also produces `tool_confirmation_event` for HITL with the same `long_running_tool_ids` shape and the same termination gap. This PR scopes to `auth_event` only. Happy to open a follow-up PR with the same fix for `tool_confirmation_event` if the team agrees with the approach here.
Co-authored-by: George Weale <gweale@google.com>
COPYBARA_INTEGRATE_REVIEW=#5638 from doughayden:fix/tool-level-auth-terminates-at-euc 0a04d30
PiperOrigin-RevId: 9327316041 parent 2e28e5d commit 883ff98
4 files changed
Lines changed: 15 additions & 16 deletions
File tree
- src/google/adk/flows/llm_flows
- tests
- integration/integrations/agent_identity
- unittests/flows/llm_flows
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1197 | 1197 | | |
1198 | 1198 | | |
1199 | 1199 | | |
| 1200 | + | |
| 1201 | + | |
| 1202 | + | |
1200 | 1203 | | |
1201 | 1204 | | |
1202 | 1205 | | |
| |||
Lines changed: 3 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
168 | 168 | | |
169 | 169 | | |
170 | 170 | | |
171 | | - | |
172 | 171 | | |
173 | 172 | | |
174 | 173 | | |
| |||
276 | 275 | | |
277 | 276 | | |
278 | 277 | | |
279 | | - | |
280 | | - | |
281 | | - | |
282 | | - | |
283 | | - | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
284 | 281 | | |
285 | 282 | | |
286 | 283 | | |
| |||
Lines changed: 3 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
159 | 159 | | |
160 | 160 | | |
161 | 161 | | |
162 | | - | |
163 | 162 | | |
164 | 163 | | |
165 | 164 | | |
| |||
266 | 265 | | |
267 | 266 | | |
268 | 267 | | |
269 | | - | |
270 | | - | |
271 | | - | |
272 | | - | |
273 | | - | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
274 | 271 | | |
275 | 272 | | |
276 | 273 | | |
| |||
Lines changed: 6 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
152 | 152 | | |
153 | 153 | | |
154 | 154 | | |
| 155 | + | |
| 156 | + | |
155 | 157 | | |
156 | 158 | | |
157 | 159 | | |
| |||
309 | 311 | | |
310 | 312 | | |
311 | 313 | | |
312 | | - | |
| 314 | + | |
313 | 315 | | |
314 | 316 | | |
315 | 317 | | |
| |||
505 | 507 | | |
506 | 508 | | |
507 | 509 | | |
508 | | - | |
| 510 | + | |
509 | 511 | | |
510 | 512 | | |
511 | 513 | | |
| |||
531 | 533 | | |
532 | 534 | | |
533 | 535 | | |
534 | | - | |
| 536 | + | |
535 | 537 | | |
536 | 538 | | |
537 | 539 | | |
| |||
550 | 552 | | |
551 | 553 | | |
552 | 554 | | |
553 | | - | |
| 555 | + | |
554 | 556 | | |
555 | 557 | | |
556 | 558 | | |
| |||
0 commit comments