From 367f0c21ed6541999ca2c362028b4896e1d51419 Mon Sep 17 00:00:00 2001 From: kshitij <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 15 Aug 2026 01:32:40 +0530 Subject: [PATCH] feat(agent): resolve sequential tool deadline via timeouts.tools.sequential_call (#85125 2a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- agent/tool_executor.py | 25 ++++++++++++++++++++- cli-config.yaml.example | 4 ++++ tests/agent/test_deadline.py | 43 ++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 1 deletion(-) diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 24bf6dbf4e..130acd5c8a 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -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, diff --git a/cli-config.yaml.example b/cli-config.yaml.example index 5f80fb23ca..37a4a0f66d 100644 --- a/cli-config.yaml.example +++ b/cli-config.yaml.example @@ -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) diff --git a/tests/agent/test_deadline.py b/tests/agent/test_deadline.py index 9e5238b9af..378a5e2c10 100644 --- a/tests/agent/test_deadline.py +++ b/tests/agent/test_deadline.py @@ -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