fix(agent): bound sequential tool calls (backport of upstream #85125) - #19
Closed
BenSheridanEdwards wants to merge 5 commits into
Closed
BenSheridanEdwards wants to merge 5 commits into
BenSheridanEdwards wants to merge 5 commits into
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. (cherry picked from commit 083f8a6)
(cherry picked from commit ededa8c)
…ential_call (NousResearch#85125 2a) Follow-up on the NousResearch#84795 salvage: the sequential deadline gets its own resolver key. Unset, it inherits the concurrent batch deadline (same value, same HERMES_CONCURRENT_TOOL_TIMEOUT_S bridge) so the two executor paths cannot drift by default; set, it can be tuned or disabled independently. Documented in cli-config.yaml.example; 5 contract tests. Deliberately NOT on run_bounded_sync: the executors extend deadlines dynamically during human approval waits (authorization-gate excluded seconds) — the shared primitive is fixed-deadline. Noted in the docstring. (cherry picked from commit 367f0c2)
When the inactivity reaper interrupts a timed-out turn, the interrupt frees the blocked frame — destroying the only evidence of where the turn was wedged. The Aug 2026 zombie-turn incident (WhatsApp session, Relay-corrupted scope stack) wedged every turn for exactly the 1800s timeout somewhere between 'Turn ended' and run_sync returning, and the wedge point was unprovable post-mortem. The reaper now logs the stack of every thread with turn-machinery frames BEFORE interrupting, so the next occurrence names the exact blocked line. Best-effort, bounded (8 threads, 25 frames), pure in-process, never raises into the reaper. (cherry picked from commit a90d536)
…out path The backported sequential-timeout path constructs _ManagedToolResult with dispatched=True. Upstream declares that field without a default and threads it through every construction site as part of a wider executor refactor that is not in this tree. Give the field a False default instead, so the existing construction sites are untouched and only the timeout path sets it explicitly. Without this, every sequential tool timeout raises TypeError instead of returning the synthesized timeout result. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
Superseded by #20 (fleet cut to v0.20.5). All five commits are subsumed by the v0.20.5 base, so nothing here needs carrying forward.
The last one is obsolete by its own reasoning: it gave If any of this was intended to reach upstream rather than just the fork, that is unaffected by closing here. |
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
4 participants
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.
Why
Three gateways (Elon, Jarvis, Neo) each lost a full agent turn on 2026-08-22. None of them crashed: the gateway processes stayed up and kept serving cron, plugin discovery and housekeeping throughout. What wedged was a single turn, and the only thing that ever ended it was the agent idle watchdog.
gateway_timeout1800sgateway_timeout1800sgateway_timeout3600sgateway_timeout3600sEvery stall ended at exactly the watchdog value, never earlier. All four were single
terminalcalls.Root cause:
execute_tool_calls_sequentialhas no deadline at all._DEFAULT_CONCURRENT_TOOL_TIMEOUT_S(420s) bounds only the concurrent batch path. A single tool call is bounded only by whatever the tool does internally, which forterminalistimeout(600s here) times up to 4 attempts via the retry loop, roughly 2414s worst case. That exceeds the 1800s watchdog outright.Ruled out along the way, for the record:
env.executeretry loop. NoExecution error, retryinglines in any stall window.What
Backport of the upstream
#85125deadline work, oldest first:083f8a6071unified deadline layer (bounded execution primitive +resolve_timeout)ededa8c4f1bound sequential tool calls — the actual fix367f0c21edresolve the sequential deadline viatimeouts.tools.sequential_calla90d5369f7dump wedged worker stacks when the turn reaper firesThe last one is diagnostics, and it is why it is here: none of the four stalls left enough evidence to identify what blocked inside the tool call. Next time it will.
Backport deltas
Two places where this tree diverges from upstream, both called out in comments:
_executor_must_emit_post_hook— upstream's version also tests_execution_dispatched, which belongs to a wider executor refactor not in this tree. Kept this tree'sagent_runtime_owns_post_tool_hook(...)condition and added only thenot _execution_timed_outguard, which is what suppresses the duplicatepost_tool_callemission on timeout._ManagedToolResult.dispatched— upstream declares it with no default and threads it through every construction site. Given aFalsedefault here so existing sites are untouched. Without it every sequential timeout raisesTypeErrorinstead of returning the synthesized result; caught bytest_sequential_tool_timeout.py.Testing
tests/agent/test_deadline.py,tests/run_agent/test_sequential_tool_timeout.py,tests/gateway/test_abandoned_turn_process_cleanup.py: 53 passedtests/agentsuite: passed (exit 0)Deploy note
Python loads modules at process start, so this is not live until gateways restart. Per-gateway:
launchctl kickstart -k gui/510/ai.hermes.gateway-<name>.Default behaviour is unchanged unless
timeouts.tools.sequential_callis set: the sequential path inherits the concurrent batch deadline, so the two executor paths cannot silently drift apart.🤖 Generated with Claude Code