fix(agent): bound synchronous tool execution with a configurable ceiling

[sylbae]

Disclosure: This PR was drafted by an AI assistant (Claude) working through a real,
interactive troubleshooting session on my self-hosted deployment. The fix was applied and
validated in production there before submission.

What does this PR do?

Bounds every synchronous tool execution with a configurable ceiling
(HERMES_TOOL_EXECUTION_CEILING_S, default 420s, 0 disables), closing a gap where the
sequential tool path (execute_tool_calls_sequential and the segmented executor's sequential
segments) has no deadline at all: a tool whose awaitable never resolves parks the conversation
turn in the event-loop selector forever — no log line, no error, ended_at/end_reason
stay NULL, and the client spins on "ruminating…" indefinitely. The concurrent path already has
HERMES_CONCURRENT_TOOL_TIMEOUT_S; this gives the synchronous funnel an equivalent backstop.

Related Issue

Fixes #79568

Type of Change

• Bug fix

Changes Made

• agent/relay_tools.py — new _tool_execution_ceiling_seconds() (env-read, default 420.0,
  warns and falls back on invalid values); _run_awaitable wraps the awaitable in
  asyncio.wait_for when the ceiling is > 0.
• tests/agent/test_relay_run_awaitable_ceiling.py — 5 new tests: normal completion,
  non-awaitable passthrough, wedged awaitable raises TimeoutError, ceiling 0 disables the
  bound, invalid env falls back to the default.

How to Test

1. pytest tests/agent/test_relay_run_awaitable_ceiling.py -v (5 passed; also executed inside
   the official nousresearch/hermes-agent:v2026.8.3 container venv, Python 3.13.5).
2. Manual: monkey-patch any tool's execute to await asyncio.Event().wait(), invoke it on the
   sequential path with HERMES_TOOL_EXECUTION_CEILING_S=5 — the tool fails visibly in ~5s and
   the turn continues, instead of hanging forever.
3. Production validation (my deployment, linux/amd64, Debian 12 host): a naturally wedged
   skill_view that previously hung the turn permanently instead surfaced as
   tool skill_view failed (420.11s); the turn issued its next API call and completed with
   finish_reason=stop.

Platforms tested: Linux (Docker, linux/amd64). No platform-specific APIs touched
(stdlib asyncio.wait_for / os.getenv only).

Checklist

• I've read the Contributing Guide
• Commit messages follow Conventional Commits
• Searched for duplicate PRs (none touch _run_awaitable or sequential-path deadlines)
• Only related commits included (single commit, single concern)
• New tests pass (5/5 in the release container venv); full suite not run locally —
  the change is additive and behind an env-tunable default
• Test coverage added for the change
• Cross-platform considered: stdlib-only, no Unix assumptions
• Documentation: env var could be added to the configuration reference — happy to include
  if maintainers point me at the right page

Screenshots / Logs

Before (production, silent forever-hang; timestamps CEST):

14:05:32 WARNING ... concurrent tool batch timed out after 120.0s; 4 tool(s) still running: ...
(no further output from this session, ever — no next API call, no error; ended_at=NULL)

After (same workload, fix applied):

17:12:44 TimeoutError
17:12:44 INFO ... tool skill_view failed (420.11s): Error executing tool 'skill_view':
17:14:56 INFO ... API call #3 ...
17:16:04 INFO ... Turn ended: reason=text_response(finish_reason=stop)

[spfcraze]

This was generated by AI during triage.

Summary:
This PR applies the ceiling in _run_awaitable, the funnel the concurrent executor's worker threads also call, so HERMES_TOOL_EXECUTION_CEILING_S bounds concurrent tools as well, on top of the separately-disableable HERMES_CONCURRENT_TOOL_TIMEOUT_S batch deadline.

Problems:

• In managed-execution mode the concurrent worker _run_tool dispatches through the same chain the sequential path uses — _run_agent_tool_execution_middleware → relay_tools.execute → _run_awaitable (agent/tool_executor.py:888 and :559, agent/relay_tools.py:114). A worker thread has no running event loop, so the new asyncio.wait_for(..., timeout=ceiling) branch applies there too. Setting HERMES_CONCURRENT_TOOL_TIMEOUT_S=0 disables the concurrent batch deadline (agent/tool_executor.py:136 returns None for value <= 0), but concurrent tools then still hit the 420s per-tool ceiling.
• The ceiling's only configuration surface is a new HERMES_* env var, which the repo's contribution rubric lists under "What we don't want" in AGENTS.md: "New HERMES_* env vars for non-secret config" — "timeouts ... go in config.yaml". The sibling HERMES_CONCURRENT_TOOL_TIMEOUT_S is env-only as well (agent/tool_executor.py:122), so the PR extends that existing pattern; the two deadlines now live on two different configuration surfaces.

Solution:
Wire the ceiling into the config schema — request_timeout_seconds and stale_timeout_seconds already live there (hermes_cli/config.py) — with the env var kept as a bridge, and add a sentence to the PR description stating the ceiling also covers the concurrent path, or scope the wait_for to the sequential funnel only.

---

Checked against 07bf13e — the tip of fix/sequential-tool-execution-ceiling when this was written — and 6564f31, main at the same moment.

[kshitijk4poor]

Salvaged via #79780 with your commit cherry-picked and authorship preserved — thank you for the excellent report and fix. The faulthandler dump in #79568 pinpointing the bare asyncio.run in relay_tools._run_awaitable, plus the production validation, made this straightforward to verify and land.

The salvage keeps your ceiling as commit 1 and adds a follow-up on top:

• a hard thread-abandon deadline (3x the ceiling) for the two wedge classes asyncio.wait_for provably can't bound — awaitables that swallow CancelledError and ones that block the event loop synchronously (wait_for joins the cancellation, so it parks in the selector exactly like the bug you fixed)
• the same bound applied to agent/relay_llm.py's byte-identical _run_awaitable twin on the managed-LLM path
• disable semantics normalized to the concurrent sibling's contract (<= 0 / NaN), and the docstring's headroom claim corrected (terminal foreground max is 600s; a slow-but-successful tool's result survives via execute()'s post-dispatch fallback, now pinned by a test)

#79780 is open for maintainer review (rebase-merge planned, so your commit lands on main under your name).

[egilewski]

suggesting changes

The configured ceiling is still cooperative rather than a hard execution bound. asyncio.wait_for cannot finish its timeout path when the wrapped awaitable swallows cancellation, while synchronous work on the event loop prevents the timeout callback from running at all. The bound probes reproduced both paths in relay_tools, so this PR does not yet enforce its claimed execution ceiling. Please supervise the work from outside its event loop so the caller can abandon the wait without depending on child cancellation, and add regression coverage for both cases.

The focused tests passed with 5 tests in 1.06 seconds, and the positive control returned normally in 0.001 seconds.

Security evidence:

• trust boundary: Relay awaitables with arbitrary tool behavior must not retain the synchronous conversation-turn runner beyond the configured bound.
• source/sink/invariant: _run_awaitable drives work through asyncio.run(asyncio.wait_for(...)); enabled bounds must return or raise without depending on event-loop progress or cooperative cancellation.
• current-main reproduction: the never-completing probe remained wedged until the 2-second outer guard exited 124.
• PR-head or patch-replay validation: cooperative non-completion raised TimeoutError in 0.101 seconds, but cancellation-swallowing work remained wedged until the 2-second outer guard and synchronous blocking returned after 1.001 seconds despite a 0.1-second ceiling.
• positive/negative cases: a quick result returned in 0.001 seconds; cooperative timeout worked, while cancellation swallowing and synchronous blocking bypassed the ceiling.
• residual bypass search: the supplied tests cover cooperative non-completion but not cancellation-resistant or event-loop-blocking awaitables.
• reviewer validation: the current-main replay passed all 5 focused tests, and dedicated probes exercised both residual wedge classes against the intended module path.

Not checked:

• Full test suite
• CodeRabbit review

Signed: GPT-5.6-sol-xhigh in Codex

[teknium1]

Closing: your commit was already carried forward (cherry-picked, authorship preserved) in the salvage PR #79780, which supersedes this branch — review continues there. Separately, the general sequential-tool gap this class of bug exposed is now bounded on main via timeouts.tools.sequential_call (#86311). Thank you for the production capture and the fix — the faulthandler dump pinpointing the bare asyncio.run in relay_tools._run_awaitable made the whole bug class tractable.