Python: fix: bound background_agents_wait_for_first_completion with a timeout - #7464
Python: fix: bound background_agents_wait_for_first_completion with a timeout#7464幻 (HUAN2022A) wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR prevents background_agents_wait_for_first_completion from potentially suspending a parent agent run indefinitely by adding a configurable timeout: a provider-level wait_timeout_seconds default and a per-call timeout_seconds override, plus new tests covering timeout behavior and defaulting.
Changes:
- Add
wait_timeout_secondstoBackgroundAgentsProvider(default 300s) and apply it inbackground_agents_wait_for_first_completion. - Add an optional
timeout_secondsparameter to the wait tool and return a status summary when the wait times out. - Add tests for timeout behavior, provider-default timeout behavior, and explicit timeout behavior when the task completes before the deadline.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| python/packages/core/agent_framework/_harness/_background_agents.py | Adds provider- and tool-level timeout support to the wait-for-completion tool, including a timeout return path that refreshes task state. |
| python/packages/core/tests/core/test_harness_background_agents.py | Adds new hanging-agent and timeout-focused tests to ensure waits are bounded and return status summaries. |
Suppressed comments (1)
python/packages/core/agent_framework/_harness/_background_agents.py:415
asyncio.waitrequires a non-negative timeout (orNone). With the new parameters, a negativetimeout_seconds(or a negative provider default) will raise and fail the tool invocation rather than returning a normal error/result string. Validate and return an error message before callingasyncio.wait.
effective_timeout = self._wait_timeout_seconds if timeout_seconds is None else timeout_seconds
done, _ = await asyncio.wait(
[t for _, t in waitable],
return_when=asyncio.FIRST_COMPLETED,
timeout=effective_timeout,
| 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, up to a timeout. | ||
|
|
||
| Returns the completed task's ID, or the current task statuses if the timeout elapses | ||
| first (in which case call this tool again or check task results to proceed). | ||
|
|
||
| Args: | ||
| task_ids: IDs of background tasks to wait on. | ||
| timeout_seconds: Maximum time to wait in seconds. When omitted, the provider's | ||
| ``wait_timeout_seconds`` is used. | ||
| """ |
There was a problem hiding this comment.
Addressed in d58c426. Omitted timeout_seconds now uses the provider default, while an explicit null/None waits indefinitely; added regression coverage for that distinction.
| self._agents = _validate_and_build_agent_dict(agents) | ||
| self._wait_timeout_seconds = wait_timeout_seconds |
There was a problem hiding this comment.
Addressed in d58c426. The provider constructor now fails fast when wait_timeout_seconds is negative, with a constructor-level regression test.
Summary
background_agents_wait_for_first_completioncould suspend the calling agent's run forever when a child task never completes. This PR adds an optional bounded timeout: await_timeout_secondsprovider default (300s) and atimeout_secondsargument on the tool itself.Nonerestores the previous unbounded behavior. When the wait elapses, the provider refreshes task state through its existing LOST-detection path and returns the current statuses to the model.Fixes #7454
Validation
ruff check,ruff format, andpyrightclean on the changed module