Python: bound background_agents_wait_for_first_completion with a timeout - #7904
Closed
Manoj Meruva (manojmeruva) wants to merge 2 commits into
Closed
Conversation
background_agents_wait_for_first_completion called asyncio.wait() with no timeout, so a child agent that never completed suspended the parent's function-calling loop indefinitely. Because the tool holds direct asyncio.Task references, _refresh_task_state never ran while the wait was parked, so a task whose runtime reference had disappeared was never promoted to LOST, and the model could not poll task status to recover. Add a provider-level wait_timeout_seconds default of 300 seconds and an optional per-call timeout_seconds override; either may be None to preserve the previous unbounded behavior. The wait now runs in bounded slices, refreshing task state between them so a LOST task ends the wait early rather than stalling for the full timeout. On timeout the tool returns current task statuses to the model instead of raising. Fixes microsoft#7454
Manoj Meruva (manojmeruva)
deployed
to
github-app-auth
August 27, 2026 09:44 — with 
GitHub Actions
Active
GitHub Actions
Active
Manoj Meruva (manojmeruva)
deployed
to
github-app-auth
August 27, 2026 09:44 — with
GitHub Actions
Active
GitHub Actions
Active
Manoj Meruva (manojmeruva)
had a problem deploying
to
github-app-auth
August 27, 2026 09:44 — with
GitHub Actions
Error
GitHub Actions
Error
agent-framework-automation
Bot
added
the
python
Manoj Meruva (manojmeruva)
deployed
to
github-app-auth
August 27, 2026 09:44 — with
GitHub Actions
Active
GitHub Actions
Active
Copilot started reviewing on behalf of
Manoj Meruva (manojmeruva)
August 27, 2026 09:44
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Adds bounded background-agent waits to prevent stalled parent runs.
Changes:
- Adds configurable provider and per-call timeouts.
- Refreshes task state during sliced waits.
- Exposes timeout configuration through the harness API and adds tests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
_background_agents.py |
Implements timeout validation, sliced waits, and status reporting. |
_agent.py |
Forwards harness timeout configuration. |
test_harness_background_agents.py |
Tests timeout, completion, validation, and lost-task behavior. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+341
to
+343
| tasks = _refresh_task_state(session, state, runtime, source_id=source_id) | ||
| if not any(t.id in waited_ids and t.status == BackgroundTaskStatus.RUNNING for t in tasks): | ||
| return set() |
Comment on lines
+185
to
+189
| if timeout_seconds != timeout_seconds: # NaN never compares greater than 0. | ||
| raise ValueError("Background agent wait timeout must not be NaN.") | ||
| if timeout_seconds <= 0: | ||
| raise ValueError(f"Background agent wait timeout must be greater than 0; got {timeout_seconds!r}.") | ||
| return float(timeout_seconds) |
| skills_paths: str | Path | Sequence[str | Path] | None = None, | ||
| background_agents: Sequence[SupportsAgentRun] | None = None, | ||
| background_agents_instructions: str | None = None, | ||
| background_agents_wait_timeout_seconds: float | None = DEFAULT_BACKGROUND_AGENTS_WAIT_TIMEOUT_SECONDS, |
Comment on lines
+654
to
+661
| if not done: | ||
| tasks = _refresh_task_state(session, provider_state, runtime, source_id=source_id) | ||
| statuses = ", ".join(f"task {t.id}: {t.status.value}" for t in tasks if t.id in task_ids) | ||
| return ( | ||
| f"Timed out after {timeout} seconds waiting for tasks {task_ids} to complete. " | ||
| f"Current status: {statuses or 'unknown'}. " | ||
| "The tasks may still be running; wait again or check their status." | ||
| ) |
Comment on lines
596
to
+600
| @tool(name="background_agents_wait_for_first_completion", approval_mode="never_require") | ||
| async def background_agents_wait_for_first_completion(task_ids: list[int]) -> str: | ||
| """Block until the first of the specified background tasks completes. Returns the completed task's ID.""" | ||
| async def background_agents_wait_for_first_completion( | ||
| task_ids: list[int], timeout_seconds: float | None = None | ||
| ) -> str: | ||
| """Block until the first of the specified background tasks completes, or the timeout elapses. |
5 tasks
- Reject non-finite timeouts. Positive infinity previously passed validation and produced an infinite deadline, restoring the unbounded wait this change exists to prevent. Convert to float first so a very large int raises the documented ValueError instead of OverflowError. - End the wait as soon as any requested task reaches a terminal state. When waiting on several IDs, one task becoming LOST was previously masked by another still running, parking the caller until the full timeout. - Distinguish a terminal-state wakeup from deadline expiry. An early return no longer reports "Timed out after N seconds" (or "after None seconds" in unbounded mode) when the deadline had not elapsed. - Bind the provider default as the tool parameter's default so an explicit timeout_seconds=None waits without a bound instead of being indistinguishable from omission. - Declare background_agents_wait_timeout_seconds in _agent.pyi so type checkers and editors accept the new keyword.
Manoj Meruva (manojmeruva)
deployed
to
github-app-auth
August 27, 2026 12:15 — with
GitHub Actions
Active
GitHub Actions
Active
Author
|
@microsoft-github-policy-service agree |
Contributor
|
Thanks for the contribution Manoj Meruva (@manojmeruva). I was actually working on a fix for this already, and it is scoped a little bit more narrowly on purpose than what you have here. Therefore closing this in favor of #7908. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
background_agents_wait_for_first_completion called asyncio.wait() with no timeout, so a child agent that never completed suspended the parent's function-calling loop indefinitely. Because the tool holds direct asyncio.Task references, _refresh_task_state never ran while the wait was parked, so a task whose runtime reference had disappeared was never promoted to LOST, and the model could not poll task status to recover.
Add a provider-level wait_timeout_seconds default of 300 seconds and an optional per-call timeout_seconds override; either may be None to preserve the previous unbounded behavior. The wait now runs in bounded slices, refreshing task state between them so a LOST task ends the wait early rather than stalling for the full timeout. On timeout the tool returns current task statuses to the model instead of raising.
Fixes #7454
Motivation & Context
background_agents_wait_for_first_completioncalledasyncio.wait()with no timeout, so a background agent that never completes suspends the calling agent's run indefinitely.The hang cannot be recovered from inside the run: the wait happens inside a tool invocation, which suspends the function-calling loop, so the model cannot check task status or take any other action while it is parked.
Description & Review Guide
What are the major changes?
BackgroundAgentsProvideracceptswait_timeout_seconds, defaulting to 300 seconds.background_agents_wait_for_first_completionaccepts an optionaltimeout_secondsthat overrides the default for a single call.Nonepreserves the previous unbounded behavior.create_harness_agentgainedbackground_agents_wait_timeout_secondsso harness users can set it.What is the impact of these changes?
Not a breaking change — the new parameters are keyword-only with defaults, and the completion path is unchanged. The intended behavioral change is that a wait which would previously block forever now returns after 300 seconds by default;
wait_timeout_seconds=Nonerestores the old behavior.What do you want reviewers to focus on?
Whether 300 seconds is the right default, and the timeout validation split: a bad provider-level value raises
ValueErrorfrom the constructor, while a bad model-suppliedtimeout_secondsis returned as an error string so a bad argument does not fail the tool invocation.Related Issue
Fixes #7454
#7464 proposed the same fix for this issue but was closed without merging. This PR implements it against current
main.Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.