feat(agent): resolve sequential tool deadline via timeouts.tools.sequential_call (#85125 2a)
Follow-up on the #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.
This commit is contained in:
@@ -666,6 +666,29 @@ def _run_agent_tool_execution_middleware(
|
||||
)
|
||||
|
||||
|
||||
def _resolve_sequential_tool_timeout() -> float | None:
|
||||
"""Deadline for one sequential tool call (#85125 Phase 2a).
|
||||
|
||||
``timeouts.tools.sequential_call`` in config.yaml wins; when unset, the
|
||||
sequential path inherits the concurrent batch deadline (same value, same
|
||||
``HERMES_CONCURRENT_TOOL_TIMEOUT_S`` legacy bridge) so the two executor
|
||||
paths cannot drift apart by default. ``0``/negative disables the bound.
|
||||
|
||||
NOTE: this path deliberately does NOT use ``agent.deadline.run_bounded_sync``.
|
||||
The sequential/concurrent executors extend their deadline dynamically while
|
||||
a human approval prompt is open (``_ConcurrentToolAuthorizationGate``
|
||||
excluded seconds — a MUST-preserve invariant) and touch agent activity
|
||||
mid-wait; the shared primitive is fixed-deadline by design. Simpler call
|
||||
sites migrate onto the primitive; these two stay symmetric with each other.
|
||||
"""
|
||||
from agent.deadline import resolve_timeout
|
||||
|
||||
return resolve_timeout(
|
||||
"tools.sequential_call",
|
||||
default=_resolve_concurrent_tool_timeout(),
|
||||
)
|
||||
|
||||
|
||||
def _run_sequential_tool_execution_middleware(
|
||||
agent,
|
||||
*,
|
||||
@@ -685,7 +708,7 @@ def _run_sequential_tool_execution_middleware(
|
||||
``<= 0``) owns that wait. Applying the generic tool deadline here would
|
||||
return ``tool_timeout`` while the prompt and worker stay active.
|
||||
"""
|
||||
timeout_s = _resolve_concurrent_tool_timeout()
|
||||
timeout_s = _resolve_sequential_tool_timeout()
|
||||
kwargs = {
|
||||
"function_name": function_name,
|
||||
"function_args": function_args,
|
||||
|
||||
@@ -181,6 +181,10 @@ model:
|
||||
# tools:
|
||||
# concurrent_batch: 420 # Deadline for a parallel tool-call batch
|
||||
# # (legacy env: HERMES_CONCURRENT_TOOL_TIMEOUT_S)
|
||||
# sequential_call: 420 # Deadline for one sequentially-executed tool call.
|
||||
# # Defaults to concurrent_batch's value so the two
|
||||
# # executor paths stay in sync; human waits
|
||||
# # (approval prompts, clarify) never count against it.
|
||||
|
||||
# =============================================================================
|
||||
# OpenRouter Provider Routing (only applies when using OpenRouter)
|
||||
|
||||
@@ -476,3 +476,46 @@ class TestConcurrentToolTimeoutMigration:
|
||||
)
|
||||
monkeypatch.setenv("HERMES_CONCURRENT_TOOL_TIMEOUT_S", "60")
|
||||
assert self._resolver()() == 300.0
|
||||
|
||||
|
||||
class TestSequentialToolTimeoutResolver:
|
||||
"""_resolve_sequential_tool_timeout: own key, inherits concurrent default."""
|
||||
|
||||
def _resolver(self):
|
||||
from agent import tool_executor
|
||||
|
||||
return tool_executor._resolve_sequential_tool_timeout
|
||||
|
||||
def test_inherits_concurrent_default(self, monkeypatch):
|
||||
monkeypatch.setattr("agent.deadline._timeouts_section", lambda: {})
|
||||
monkeypatch.delenv("HERMES_CONCURRENT_TOOL_TIMEOUT_S", raising=False)
|
||||
assert self._resolver()() == 420.0
|
||||
|
||||
def test_inherits_concurrent_env_bridge(self, monkeypatch):
|
||||
# No sequential-specific setting -> concurrent env var flows through.
|
||||
monkeypatch.setattr("agent.deadline._timeouts_section", lambda: {})
|
||||
monkeypatch.setenv("HERMES_CONCURRENT_TOOL_TIMEOUT_S", "60")
|
||||
assert self._resolver()() == 60.0
|
||||
|
||||
def test_own_config_key_wins_over_concurrent(self, monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
"agent.deadline._timeouts_section",
|
||||
lambda: {"tools": {"concurrent_batch": 300, "sequential_call": 90}},
|
||||
)
|
||||
assert self._resolver()() == 90.0
|
||||
|
||||
def test_zero_disables_independently(self, monkeypatch):
|
||||
# Sequential bound can be disabled while the concurrent one stays on.
|
||||
monkeypatch.setattr(
|
||||
"agent.deadline._timeouts_section",
|
||||
lambda: {"tools": {"concurrent_batch": 300, "sequential_call": 0}},
|
||||
)
|
||||
assert self._resolver()() is None
|
||||
|
||||
def test_concurrent_disabled_flows_through(self, monkeypatch):
|
||||
# concurrent disabled (None default) + no sequential key -> unbounded.
|
||||
monkeypatch.setattr(
|
||||
"agent.deadline._timeouts_section",
|
||||
lambda: {"tools": {"concurrent_batch": 0}},
|
||||
)
|
||||
assert self._resolver()() is None
|
||||
|
||||
Reference in New Issue
Block a user