Commit 812a7dc
fix(providers): suppress idle watchdog during in-flight Copilot tool calls (#490)
* fix(providers): suppress idle watchdog during in-flight Copilot tool calls
The Copilot SDK emits no events between tool.execution_start and
tool.execution_complete, so a stale idle clock during a long-running
tool call was indistinguishable from a genuinely stuck session,
triggering a spurious "please continue" recovery prompt mid tool-call
that could overwrite the agent's eventual structured output.
In-flight tool calls (tracked by tool_call_id) now suppress idle
recovery entirely while any remain outstanding; max_session_seconds /
max_agent_iterations remain the backstop for a genuinely wedged tool.
last_activity_ref's tool name is cleared (or rolled to another
still-in-flight tool) on tool.execution_complete instead of only ever
being set.
Adds configurable runtime.idle_timeout_seconds and
runtime.max_idle_recovery_attempts (Copilot-only) so workflows with
legitimately long tool calls can tune the watchdog.
Closes #488
* fix(providers): address PR #490 review findings on idle watchdog suppression
Blocking fixes (#488):
- Remove the "pop the oldest entry" fallback in the tool.execution_complete
handler that could evict a different, still-running tool's active_tools
entry on a duplicate/unmatched event, re-arming the watchdog mid-tool-call
and reproducing #488 while appearing fixed. Replaced with a non-mutating
debug log; max_session_seconds remains the backstop for a stale entry.
- Strengthen TestOnEventActiveTools assertions so both tests actually pin
the fix (verified to fail against the pre-fix provider, pass post-fix).
- Add an end-to-end regression test driving a real tool.execution_start ->
silence -> tool.execution_complete sequence through _send_and_wait,
asserting the recovery prompt never clobbers response_content.
- Add a warn-once latch (mirroring _context_window_anomaly_warned) so the
first occurrence of extended idle-recovery suppression during a session
is logged at warning level (console + logger), instead of silently
degrading a previously console-visible 90s warning into up to 31.5
minutes of total silence.
Recommendations applied:
- Corrected the repeated false claim that the SDK "emits no events" during
a tool call (it does not guarantee any, but tool.execution_progress /
tool.execution_partial_result exist and are opt-in) across copilot.py,
schema.py, docs/configuration.md, CHANGELOG.md, and the PR description;
consolidated the rationale into one canonical docstring.
- Corrected the inaccurate claim that max_agent_iterations backstops a
wedged tool call (its counter only advances on tool.execution_start, so
it's frozen for the whole wedge) — max_session_seconds is the sole
backstop.
- Added IdleRecoveryConfig.__post_init__ validation so directly-constructed
configs (bypassing the Pydantic schema bounds) can't produce an unbounded
busy-wait loop.
- Simplified factory.py's IdleRecoveryConfig construction to a dict-filter
+ single constructor call instead of a three-way ternary per field.
- Added ProviderCapabilities.idle_recovery (Copilot-only) with a
workflow-level validator warning (not an error, since these are tuning
knobs rather than safety bounds) when idle_timeout_seconds /
max_idle_recovery_attempts are set against a provider that ignores them.
- Bounded two previously-unbounded busy-wait test loops with
asyncio.wait_for(..., timeout=5.0).
- Added an overlapping-tool-calls end-to-end test keyed on tool_call_id
(not tool_name), verified to reproduce the hang if the dict were
mistakenly keyed by tool name instead.
- Documented the max_session_seconds backstop in docs/configuration.md so
a legitimately long tool call doesn't silently exceed it unexpectedly.
Skipped: ACA forwarding of the two idle-recovery fields (larger, separate
scope spanning factory/aca/aca_runner) and AGENTS.md documentation update
(the two runtime knobs and active_tools mechanism are already documented
in the config docs and code comments; deferring to keep this diff scoped
to the review findings).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Jason Robert <jasonrobert@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>1 parent 75eceda commit 812a7dc
16 files changed
Lines changed: 1014 additions & 6 deletions
File tree
- docs
- plugins/conductor/skills/conductor/references
- src/conductor
- config
- providers
- tests
- test_config
- test_providers
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
25 | 34 | | |
26 | 35 | | |
27 | 36 | | |
| |||
34 | 43 | | |
35 | 44 | | |
36 | 45 | | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
37 | 67 | | |
38 | 68 | | |
39 | 69 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
| 20 | + | |
| 21 | + | |
20 | 22 | | |
21 | 23 | | |
22 | 24 | | |
| |||
31 | 33 | | |
32 | 34 | | |
33 | 35 | | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
34 | 50 | | |
35 | 51 | | |
36 | 52 | | |
| |||
47 | 63 | | |
48 | 64 | | |
49 | 65 | | |
| 66 | + | |
| 67 | + | |
50 | 68 | | |
51 | 69 | | |
52 | 70 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
75 | 75 | | |
76 | 76 | | |
77 | 77 | | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
78 | 86 | | |
79 | 87 | | |
80 | 88 | | |
| |||
Lines changed: 2 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
35 | 35 | | |
36 | 36 | | |
37 | 37 | | |
| 38 | + | |
| 39 | + | |
38 | 40 | | |
39 | 41 | | |
40 | 42 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3419 | 3419 | | |
3420 | 3420 | | |
3421 | 3421 | | |
| 3422 | + | |
| 3423 | + | |
| 3424 | + | |
| 3425 | + | |
| 3426 | + | |
| 3427 | + | |
| 3428 | + | |
| 3429 | + | |
| 3430 | + | |
| 3431 | + | |
| 3432 | + | |
| 3433 | + | |
| 3434 | + | |
| 3435 | + | |
| 3436 | + | |
| 3437 | + | |
| 3438 | + | |
| 3439 | + | |
| 3440 | + | |
| 3441 | + | |
| 3442 | + | |
| 3443 | + | |
3422 | 3444 | | |
3423 | 3445 | | |
3424 | 3446 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1821 | 1821 | | |
1822 | 1822 | | |
1823 | 1823 | | |
| 1824 | + | |
| 1825 | + | |
1824 | 1826 | | |
1825 | 1827 | | |
1826 | 1828 | | |
| |||
2466 | 2468 | | |
2467 | 2469 | | |
2468 | 2470 | | |
| 2471 | + | |
| 2472 | + | |
| 2473 | + | |
| 2474 | + | |
| 2475 | + | |
| 2476 | + | |
| 2477 | + | |
| 2478 | + | |
| 2479 | + | |
| 2480 | + | |
| 2481 | + | |
| 2482 | + | |
| 2483 | + | |
| 2484 | + | |
| 2485 | + | |
| 2486 | + | |
| 2487 | + | |
| 2488 | + | |
| 2489 | + | |
| 2490 | + | |
| 2491 | + | |
| 2492 | + | |
| 2493 | + | |
| 2494 | + | |
| 2495 | + | |
| 2496 | + | |
| 2497 | + | |
| 2498 | + | |
| 2499 | + | |
| 2500 | + | |
2469 | 2501 | | |
2470 | 2502 | | |
2471 | 2503 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
192 | 192 | | |
193 | 193 | | |
194 | 194 | | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
195 | 204 | | |
196 | 205 | | |
197 | 206 | | |
| |||
274 | 283 | | |
275 | 284 | | |
276 | 285 | | |
| 286 | + | |
| 287 | + | |
277 | 288 | | |
278 | 289 | | |
279 | 290 | | |
| |||
0 commit comments