feat(agent): unified deadline layer — bounded execution primitive + timeout resolver (#85125 Phase 1) - #85147
Merged
kshitijk4poor merged 3 commits intoAug 14, 2026
Conversation
…imeout resolver (NousResearch#85125 Phase 1) One shared foundation for the timeout/hang backlog instead of per-incident site-local fixes: - agent/deadline.py: run_bounded_async (thread-timer deadline that survives a blocked event loop, generalizing the telegram adapter primitive), run_bounded_sync, clamp_timeout (kills the NousResearch#83220 time_t OverflowError class at the boundary), resolve_timeout (config.yaml timeouts: section > legacy env bridge > default), kill_process_tree (whole-tree termination for the NousResearch#71148 orphan class), DeadlineExpired (our deadline, mechanically distinct from provider timeouts). - tool_executor._resolve_concurrent_tool_timeout migrates onto the resolver; exact legacy env-var contract preserved (default 420, 0 disables). - timeouts: accepted as a known config root; documented in cli-config.yaml.example. Pure addition otherwise — no behavior change, no new env vars, no cache impact. Later phases (NousResearch#85125) migrate tool-execution, MCP, and subprocess call sites onto these primitives.
- run_bounded_async: cancel + abandon the inner task when the CALLER is cancelled (leak the telegram original also had) - kill_process_tree: check taskkill exit code (Windows contract parity), suppress console flash via windows_hide_flags, and sweep a psutil descendant snapshot taken before signalling — reaches grandchildren in their own setsid sessions and the non-group-leader case (NousResearch#71148 class) - resolve_timeout: reject bool (YAML true would become a 1s deadline) and NaN config values with fall-through instead of resolving unbounded - BoundedResult: kw_only to prevent positional transposition - tests: real clamped-value time_t regression proof, own-session descendant kill, external-cancellation task cleanup, bool/NaN config fall-through; pin already-dead-pid contract
The os.killpg call sits below an early 'if sys.platform == win32: return' so it can never execute on Windows; the scanner is line-based and needs the inline marker.
Open
1 task
kshitijk4poor
marked this pull request as draft
August 13, 2026 08:30
kshitijk4poor
marked this pull request as ready for review
August 14, 2026 19:40
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdded a shared deadline module for timeout resolution, bounded async and sync execution, diagnostics, and process-tree termination. Registered timeout configuration, documented precedence rules, migrated concurrent tool timeout handling, and added comprehensive tests. ChangesUnified deadline infrastructure
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant run_bounded_async
participant DaemonTimer
participant asyncio.Task
Caller->>run_bounded_async: Start awaitable with timeout
run_bounded_async->>DaemonTimer: Schedule independent deadline
DaemonTimer->>run_bounded_async: Signal timeout
run_bounded_async->>asyncio.Task: Cancel or abandon task
run_bounded_async-->>Caller: Return BoundedResult
Suggested reviewers: ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This was referenced Aug 20, 2026
ayushnangia
added a commit
to ayushnangia/hermes-agent
that referenced
this pull request
Aug 24, 2026
…ll_process_tree The script-timeout path used a site-local process-group kill, which cannot reach a grandchild that created its OWN session (start_new_session background jobs, watchdogs). Such descendants kept running after the job reported failure (NousResearch#71148, NousResearch#59549). Migrate the timeout handler to the unified deadline layer's kill_process_tree (NousResearch#85147, d6a5cb9): psutil snapshots the descendant set before signalling, so own-session grandchildren are reached too. Fallback to the site-local group kill if the import ever fails, so the path cannot re-wedge. The explicit script-timeout message stays the classification anchor (NousResearch#85536's contract), keeping cron timeouts distinct from provider timeouts. Co-authored-by: dante32683 <dante32683@users.noreply.github.com> Co-authored-by: supotato-ipj <supotato-ipj@users.noreply.github.com>
kshitijk4poor
added a commit
that referenced
this pull request
Aug 24, 2026
…adline.run_bounded_async (#85125 2f) The adapter's private thread-deadline helper was the ancestor of the unified deadline layer's run_bounded_async (#85147 was extracted from it, plus the caller-cancellation leak fix the original still lacked). Consolidate: the helper body becomes a thin wrapper mapping BoundedResult.timed_out back to the asyncio.TimeoutError its 9 call sites (the PTB retry ladder) expect. ~90 duplicated lines die, along with the adapter-local copies of the abandon-cleanup runner and the blocked-loop faulthandler diagnostics (both live in agent/deadline.py). Everything the call sites rely on is preserved by the unified layer: - thread-timer deadline that survives a blocked event loop (#63309) - abandonment of cancellation-shielded tasks (PTB/httpcore anyio init) - detached best-effort on_abandon cleanup (no httpx pool leak per retry) - off-loop stack dump when the loop never processes the expiry Plus one behavior IMPROVEMENT inherited from the shared copy: a caller cancelling the wrapper no longer leaks the inner task unobserved (the telegram original had that leak; the extraction fixed it). test_telegram_init_deadline.py: the #63309 diagnostics probe now pins the shared layer's dump hook (label "telegram-init") — same contract, new seam. Wedge + cleanup-crash tests pass unchanged.
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.
Summary
Phase 1 of #85125: one shared deadline layer (
agent/deadline.py) so the timeout/hang bug classes get fixed once at a primitive instead of once per incident — plus the first call-site migration proving the pattern.Changes
agent/deadline.py(new):run_bounded_async— wall-clock deadline for awaitables driven by a daemonthreading.Timer, so a blocked event loop cannot disable the deadline (family A of the Triage: the seven mechanisms behind the 77 open stall/hang reports #84047 stall triage; generalizes the proven telegram-adapter_await_with_thread_deadlineprimitive, including the blocked-loop stack-dump watchdog and abandonment of cancellation-shielded tasks)run_bounded_sync— same contract for sync callables (daemon worker, abandoned on expiry,on_timeouthook for marking backends suspect)clamp_timeout— platform-safe clamping; kills theOverflowError: timestamp out of range for platform time_tclass ([Bug] Large approvals.timeout crashes all parallel tool calls on macOS (OverflowError: timestamp out of range for platform time_t) #83220) at the shared boundaryresolve_timeout— one resolution path:timeouts:section in config.yaml > legacyHERMES_*env bridge > default. No new user-facing env vars (".env is for secrets only")kill_process_tree— portable whole-tree termination (POSIXkillpgfor session leaders / Windowstaskkill /F /T) for the orphaned-process-tree class (Cron script timeout leaves orphaned process trees (timeout kill doesn't reach descendants) #71148, [Bug]: Cron script timeouts leave orphaned background processes and get reported as provider timeouts #59549, [Bug]: Docker terminal timeout leaves in-container process trees running #84967, browser_tool: command timeout leaks the spawned agent-browser daemon + Chromium tree (long-lived gateway accumulates orphans until host OOM) #68139)DeadlineExpired— our deadline, mechanically distinct from provider/transport timeouts (the [Bug]: Cron script timeouts leave orphaned background processes and get reported as provider timeouts #59549/Cron no_agent script failure reported as 'provider timeout / fallback chain exhausted' — misleading error message #80323 misattribution class)agent/tool_executor.py—_resolve_concurrent_tool_timeout()migrates ontoresolve_timeout("tools.concurrent_batch"); exact legacy contract preserved (default 420s,HERMES_CONCURRENT_TOOL_TIMEOUT_Sbridge,0disables)hermes_cli/config.py—timeoutsregistered as a known config rootcli-config.yaml.example— documents the new sectiontests/agent/test_deadline.py— 40 tests: clamp normalization (including a real time_t regression proof for the [Bug] Large approvals.timeout crashes all parallel tool calls on macOS (OverflowError: timestamp out of range for platform time_t) #83220 class), resolver precedence + bool/NaN rejection, sync/async bounding (cancellation-shielded hung task, external-cancellation cleanup), grandchild and own-session-descendant process-tree kills, and the tool_executor back-compat contractValidation
tests/agent/test_deadline.pytests/run_agent/executor-adjacent (-k 'concurrent or batch or timeout or executor')tests/run_agent/test_start_order_gate.py+tests/hermes_cli/test_config.pyHERMES_HOME, real config.yaml)model_tools+agent.tool_executorwith worktree on sys.path)Self-review (3-angle + Hermes-specific) applied before push: caller-cancellation task leak fixed, taskkill exit-code contract, psutil descendant sweep for own-session grandchildren, bool/NaN config rejection, kw_only BoundedResult. Known deliberate deferrals, tracked in #85125: telegram's private
_await_with_thread_deadlinecopy migrates in Phase 2;gateway/status.pyandtools/code_execution_tool.pytree-kill sites migrate in Phase 4 (both named in the module docstring).Pure addition otherwise: no behavior change under default config, no cache impact, no message-flow changes. Later phases of #85125 migrate the sequential tool path, MCP handlers, and subprocess kill sites onto these primitives (aligning in-flight PRs #84795 / #84125 / #76822).
Part of #85125.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes