feat(agents): inherit tool timeout ceilings from parent LLM agent #141

Merged
CoreRasurae merged 1 commit from feature/m1-inherit-tool-timeout-ceilings into master 2026-08-24 10:58:35 +00:00
Member

Summary

  • A type: llm agent's tools_max_timeout/shell_max_timeout config now bounds the per-invocation timeout tool-call argument for shell/http_request calls that agent dispatches internally, exactly as it already does for a directly-configured type: tool agent. timeout itself already inherited correctly and is untouched.
  • All three ephemeral-ToolAgent-construction sites in LLMAgent._execute_tool_loop now share one _build_ephemeral_tool_config() helper (Template/Factory Method) instead of three near-identical inline dict literals, closing the drift that let tools_max_timeout/shell_max_timeout diverge from timeout in the first place.
  • Extends docs/adr/ADR-2030-tool-calling-spec-extensions.md with D-10 and docs/index.md §4.4 (Version 1.6.0, §21.1 Revision History) per the spec-revision procedure, since this makes the two fields usable/discoverable on a type: llm agent for the first time.
  • The per-invocation timeout tool-call argument mechanism (ADR-2030 D-9, TimeoutPolicy) is unaffected: a model-supplied override still takes precedence over the (now possibly inherited) default/ceiling for that one call.

Closes #140

Test plan

  • nox -s lint / nox -s format -- --check — green
  • nox -s typecheck — green (0 errors)
  • nox -s security_scan / nox -s dead_code — green, no findings
  • nox -s unit_tests — 154 features, 3089 scenarios, 0 failed (6 new scenarios in features/llm_agent_tool_loop.feature)
  • nox -s coverage_report — 96.9% (threshold 96.5%); no new uncovered lines
  • nox -s integration_tests — 364 Robot tests, 0 failed (2 new tests in robot/llm_tool_calling.robot)
  • nox -s benchmark_regression -- --quick — BENCHMARKS NOT SIGNIFICANTLY CHANGED (expected: config-threading only, no perf-sensitive path touched)
## Summary - A `type: llm` agent's `tools_max_timeout`/`shell_max_timeout` config now bounds the per-invocation `timeout` tool-call argument for `shell`/`http_request` calls that agent dispatches internally, exactly as it already does for a directly-configured `type: tool` agent. `timeout` itself already inherited correctly and is untouched. - All three ephemeral-`ToolAgent`-construction sites in `LLMAgent._execute_tool_loop` now share one `_build_ephemeral_tool_config()` helper (Template/Factory Method) instead of three near-identical inline dict literals, closing the drift that let `tools_max_timeout`/`shell_max_timeout` diverge from `timeout` in the first place. - Extends `docs/adr/ADR-2030-tool-calling-spec-extensions.md` with D-10 and `docs/index.md` §4.4 (Version 1.6.0, §21.1 Revision History) per the spec-revision procedure, since this makes the two fields usable/discoverable on a `type: llm` agent for the first time. - The per-invocation `timeout` tool-call argument mechanism (ADR-2030 D-9, `TimeoutPolicy`) is unaffected: a model-supplied override still takes precedence over the (now possibly inherited) default/ceiling for that one call. Closes #140 ## Test plan - [x] `nox -s lint` / `nox -s format -- --check` — green - [x] `nox -s typecheck` — green (0 errors) - [x] `nox -s security_scan` / `nox -s dead_code` — green, no findings - [x] `nox -s unit_tests` — 154 features, 3089 scenarios, 0 failed (6 new scenarios in `features/llm_agent_tool_loop.feature`) - [x] `nox -s coverage_report` — 96.9% (threshold 96.5%); no new uncovered lines - [x] `nox -s integration_tests` — 364 Robot tests, 0 failed (2 new tests in `robot/llm_tool_calling.robot`) - [x] `nox -s benchmark_regression -- --quick` — BENCHMARKS NOT SIGNIFICANTLY CHANGED (expected: config-threading only, no perf-sensitive path touched)
CoreRasurae added this to the v2.1.0 milestone 2026-08-21 16:59:48 +00:00
feat(agents): inherit tool timeout ceilings from parent LLM agent
Some checks failed
CI / lint (pull_request) Successful in 51s
CI / typecheck (pull_request) Successful in 1m27s
CI / unit_tests (pull_request) Has started running
CI / benchmark (pull_request) Has started running
CI / quality (pull_request) Successful in 1m12s
CI / security (pull_request) Successful in 2m40s
CI / build (pull_request) Successful in 1m50s
CI / integration_tests (pull_request) Successful in 2m48s
CI / coverage (pull_request) Has been cancelled
CI / status-check (pull_request) Has been cancelled
b834a518f2
A type: llm agent dispatches its built-in shell/http_request tool calls
through an ephemeral, per-call ToolAgent it constructs internally. That
ephemeral agent's `timeout` default already came from the parent LLM
agent's own config; `tools_max_timeout`/`shell_max_timeout` did not,
so every LLM-agent-dispatched tool call was capped at the ephemeral
agent's built-in 120s ceiling no matter what the parent agent declared.

All three tool-call dispatch sites in LLMAgent._execute_tool_loop now
share a new _build_ephemeral_tool_config() helper that forwards
tools_max_timeout/shell_max_timeout from the parent's config (falling
back to ToolAgent's own defaults when absent), replacing three
near-identical inline dict literals that had already let the ceiling
fields drift out of sync with timeout. The per-invocation timeout
tool-call argument (ADR-2030 D-9) is unaffected: a model-supplied
override still takes precedence over the (now possibly inherited)
default and ceiling for that one call.

Extends docs/adr/ADR-2030-tool-calling-spec-extensions.md with D-10
and docs/index.md §4.4 (Version 1.6.0) per the spec-revision procedure,
since this makes tools_max_timeout/shell_max_timeout usable/discoverable
on a type: llm agent for the first time.

ISSUES CLOSED: #140
docs(tools): document type: llm agents honoring tools_max_timeout
Some checks failed
CI / lint (pull_request) Successful in 37s
CI / integration_tests (pull_request) Has started running
CI / benchmark (pull_request) Has started running
CI / typecheck (pull_request) Successful in 1m27s
CI / quality (pull_request) Successful in 1m29s
CI / build (pull_request) Successful in 36s
CI / security (pull_request) Successful in 1m46s
CI / coverage (pull_request) Has been cancelled
CI / status-check (pull_request) Has been cancelled
CI / unit_tests (pull_request) Has been cancelled
9eb8378d64
The tool-agent user guide (docs/tools/timeouts.md, index.md,
built-in-tools.md) only described tools_max_timeout/shell_max_timeout
in the context of a type: tool agent. Adds a section and cross-links
covering the ADR-2030 D-10 behavior: a type: llm agent's own config
now accepts the same three fields for tool calls it dispatches
internally.

Refs: #140
CoreRasurae force-pushed feature/m1-inherit-tool-timeout-ceilings from 9eb8378d64
Some checks failed
CI / lint (pull_request) Successful in 37s
CI / integration_tests (pull_request) Has started running
CI / benchmark (pull_request) Has started running
CI / typecheck (pull_request) Successful in 1m27s
CI / quality (pull_request) Successful in 1m29s
CI / build (pull_request) Successful in 36s
CI / security (pull_request) Successful in 1m46s
CI / coverage (pull_request) Has been cancelled
CI / status-check (pull_request) Has been cancelled
CI / unit_tests (pull_request) Has been cancelled
to 49e7e263a0
Some checks failed
CI / lint (pull_request) Successful in 39s
CI / quality (pull_request) Successful in 55s
CI / security (pull_request) Successful in 1m21s
CI / typecheck (pull_request) Successful in 1m36s
CI / build (pull_request) Successful in 38s
CI / integration_tests (pull_request) Successful in 2m7s
CI / unit_tests (pull_request) Successful in 5m28s
CI / coverage (pull_request) Failing after 11m52s
CI / benchmark (pull_request) Failing after 16m36s
CI / status-check (pull_request) Has been cancelled
2026-08-21 17:09:31 +00:00
Compare
hurui200320 left a comment

PR Review: !141 (Ticket #140)

Verdict: Approve

The implementation correctly threads tools_max_timeout and shell_max_timeout from a parent type: llm agent into the ephemeral ToolAgent instances it constructs per dispatched tool call, satisfying the functional requirements of #140. The code is clean, well-documented, and the spec/ADR updates are consistent. No critical or major issues were found.

Critical Issues

None

Major Issues

None

Minor Issues

  1. Missing Behave coverage for the post-loop stuck-model synthesis dispatch site

    • File: features/llm_agent_tool_loop.feature
    • Line: 177-182
    • Problem: The acceptance criteria require coverage "across all three dispatch sites" (normal round, budget-exhaustion synthesis, post-loop synthesis). The PR adds a scenario for the budget-exhaustion site, but the post-loop stuck-model synthesis site is not explicitly exercised. Since all three sites now call the shared _build_ephemeral_tool_config() helper, the functional risk is low, but the requirement coverage is incomplete.
    • Recommendation: Add a Behave scenario that forces the post-loop stuck-model synthesis path and asserts that the inherited tools_max_timeout ceiling is honored there.
  2. Invalid ceiling test only exercises tools_max_timeout: 0

    • File: features/llm_agent_tool_loop.feature
    • Line: 170-175
    • Problem: The acceptance criteria state that non-numeric, zero, negative, and non-finite values for either tools_max_timeout or shell_max_timeout must raise AgentCreationError at ephemeral-agent construction. The new scenario only tests tools_max_timeout: 0; it does not cover non-numeric, negative, non-finite, or shell_max_timeout invalid values.
    • Recommendation: Expand the invalid-config coverage with parameterized or additional scenarios for negative, non-numeric, non-finite, and shell_max_timeout cases.
  3. Robot integration tests do not exercise inherited shell_max_timeout

    • File: robot/llm_tool_calling.robot
    • Line: 107-123
    • Problem: The Robot suite verifies tools_max_timeout inheritance but does not include a case for shell_max_timeout overriding the shell-only ceiling.
    • Recommendation: Add a Robot test that configures shell_max_timeout on the LLM agent and verifies a per-call timeout above tools_max_timeout but below shell_max_timeout is accepted for a shell call.

Nits

  1. Hardcoded fallback values mirror ToolAgent defaults
    • File: src/cleveractors/agents/llm.py
    • Line: 978-979
    • Problem: _build_ephemeral_tool_config() hardcodes 120.0 and None as fallbacks. These match ToolAgent.__init__ defaults today, but if those defaults ever change, this helper could silently drift out of sync.
    • Recommendation: Consider referencing ToolAgent defaults via module constants or a shared helper to make the coupling explicit. (This is consistent with the existing timeout: 1 pattern, so purely a nit.)

Summary

This is a focused, well-scoped change. Extracting the ephemeral-tool-agent config construction into _build_ephemeral_tool_config() removes the drift that allowed the timeout ceilings to diverge, and the ADR/spec updates follow the established process. The implementation is functionally correct and backward-compatible. Addressing the minor test-coverage gaps would make the PR fully satisfy the acceptance criteria, but the current state is safe to approve.

## PR Review: !141 (Ticket #140) ### Verdict: Approve The implementation correctly threads `tools_max_timeout` and `shell_max_timeout` from a parent `type: llm` agent into the ephemeral `ToolAgent` instances it constructs per dispatched tool call, satisfying the functional requirements of #140. The code is clean, well-documented, and the spec/ADR updates are consistent. No critical or major issues were found. ### Critical Issues None ### Major Issues None ### Minor Issues 1. **Missing Behave coverage for the post-loop stuck-model synthesis dispatch site** - **File:** `features/llm_agent_tool_loop.feature` - **Line:** 177-182 - **Problem:** The acceptance criteria require coverage "across all three dispatch sites" (normal round, budget-exhaustion synthesis, post-loop synthesis). The PR adds a scenario for the budget-exhaustion site, but the post-loop stuck-model synthesis site is not explicitly exercised. Since all three sites now call the shared `_build_ephemeral_tool_config()` helper, the functional risk is low, but the requirement coverage is incomplete. - **Recommendation:** Add a Behave scenario that forces the post-loop stuck-model synthesis path and asserts that the inherited `tools_max_timeout` ceiling is honored there. 2. **Invalid ceiling test only exercises `tools_max_timeout: 0`** - **File:** `features/llm_agent_tool_loop.feature` - **Line:** 170-175 - **Problem:** The acceptance criteria state that non-numeric, zero, negative, and non-finite values for either `tools_max_timeout` or `shell_max_timeout` must raise `AgentCreationError` at ephemeral-agent construction. The new scenario only tests `tools_max_timeout: 0`; it does not cover non-numeric, negative, non-finite, or `shell_max_timeout` invalid values. - **Recommendation:** Expand the invalid-config coverage with parameterized or additional scenarios for negative, non-numeric, non-finite, and `shell_max_timeout` cases. 3. **Robot integration tests do not exercise inherited `shell_max_timeout`** - **File:** `robot/llm_tool_calling.robot` - **Line:** 107-123 - **Problem:** The Robot suite verifies `tools_max_timeout` inheritance but does not include a case for `shell_max_timeout` overriding the shell-only ceiling. - **Recommendation:** Add a Robot test that configures `shell_max_timeout` on the LLM agent and verifies a per-call `timeout` above `tools_max_timeout` but below `shell_max_timeout` is accepted for a `shell` call. ### Nits 1. **Hardcoded fallback values mirror `ToolAgent` defaults** - **File:** `src/cleveractors/agents/llm.py` - **Line:** 978-979 - **Problem:** `_build_ephemeral_tool_config()` hardcodes `120.0` and `None` as fallbacks. These match `ToolAgent.__init__` defaults today, but if those defaults ever change, this helper could silently drift out of sync. - **Recommendation:** Consider referencing `ToolAgent` defaults via module constants or a shared helper to make the coupling explicit. (This is consistent with the existing `timeout: 1` pattern, so purely a nit.) ### Summary This is a focused, well-scoped change. Extracting the ephemeral-tool-agent config construction into `_build_ephemeral_tool_config()` removes the drift that allowed the timeout ceilings to diverge, and the ADR/spec updates follow the established process. The implementation is functionally correct and backward-compatible. Addressing the minor test-coverage gaps would make the PR fully satisfy the acceptance criteria, but the current state is safe to approve.
CoreRasurae force-pushed feature/m1-inherit-tool-timeout-ceilings from 49e7e263a0
Some checks failed
CI / lint (pull_request) Successful in 39s
CI / quality (pull_request) Successful in 55s
CI / security (pull_request) Successful in 1m21s
CI / typecheck (pull_request) Successful in 1m36s
CI / build (pull_request) Successful in 38s
CI / integration_tests (pull_request) Successful in 2m7s
CI / unit_tests (pull_request) Successful in 5m28s
CI / coverage (pull_request) Failing after 11m52s
CI / benchmark (pull_request) Failing after 16m36s
CI / status-check (pull_request) Has been cancelled
to cb48550f16
Some checks failed
CI / lint (pull_request) Successful in 1m58s
CI / typecheck (pull_request) Successful in 1m59s
CI / unit_tests (pull_request) Has started running
CI / integration_tests (pull_request) Has started running
CI / security (pull_request) Successful in 1m0s
CI / benchmark (pull_request) Has started running
CI / build (pull_request) Successful in 38s
CI / coverage (pull_request) Has been cancelled
CI / status-check (pull_request) Has been cancelled
CI / quality (pull_request) Has been cancelled
2026-08-24 10:40:09 +00:00
Compare
Author
Member

@hurui200320 Thanks for the review — all three Minor items and the Nit are addressed in cb48550 (force-pushed to this branch). Details below, checked against docs/index.md §4.4/§4.5, ADR-2030 D-9/D-10, and issue #140's acceptance criteria before applying.

Minor 1 — Missing Behave coverage for the post-loop stuck-model synthesis dispatch site

Done. Added a scenario ("A post-loop stuck-model synthesis tool dispatch also honors an inherited tools_max_timeout ceiling") in features/llm_agent_tool_loop.feature that drives the loop to the third _build_ephemeral_tool_config() call site (tool_config_s, the post-loop synthesis dispatch) with a shell call carrying a timeout above the built-in default but within an inherited tools_max_timeout: 300, and asserts it succeeds. This now gives Behave coverage across all three dispatch sites named in #140's acceptance criteria (normal round — already covered; budget-exhaustion synthesis — already covered; post-loop synthesis — new).

Minor 2 — Invalid ceiling test only exercised tools_max_timeout: 0

Done. Added three scenarios alongside the existing tools_max_timeout: 0 case:

  • negative tools_max_timeout (-5)
  • non-numeric tools_max_timeout (not_a_number)
  • non-finite tools_max_timeout (nan)
  • invalid shell_max_timeout (-5, with a valid tools_max_timeout)

All four assert AgentCreationError at ephemeral-agent construction, matching #140's "non-numeric, zero, negative, or non-finite... for either tools_max_timeout or shell_max_timeout" criterion. (Implementation note: the negative cases reuse the existing integer-typed Given steps rather than new step definitions, since -5 is representable there directly — no behavioral difference, just avoided adding redundant step functions.)

Minor 3 — Robot integration tests do not exercise inherited shell_max_timeout

Done. Added LLM Agent's shell_max_timeout Raises The Shell-Only Ceiling For A Dispatched Shell Call (Ticket 140) to robot/llm_tool_calling.robot, using the shell_max_timeout parameter already wired into Create Executor With Shell Tool Calling Agent (robot/ToolCallingTestLib.py). It configures tools_max_timeout=2/shell_max_timeout=100 and issues a per-call timeout=50 (above the generic ceiling, below the shell-only one) — exactly the case you suggested.

Nit — Hardcoded fallback values mirror ToolAgent defaults

Done. Extracted DEFAULT_TOOLS_MAX_TIMEOUT = 120.0 and DEFAULT_SHELL_MAX_TIMEOUT = None as module-level constants in src/cleveractors/agents/tool.py, and both ToolAgent.__init__ and LLMAgent._build_ephemeral_tool_config() now read from them instead of each hardcoding the literals independently. This closes the exact drift risk you flagged — the two call sites can no longer diverge silently if the default ever changes.

Not changed

Nothing from your review was skipped or rejected — all four items were verified as correct against the spec/ADR/issue before being applied, and all are now in the diff.

Verification

  • nox -s lint / format --check — green
  • nox -s typecheck — 0 errors
  • nox -s unit_tests — 3094 scenarios passed (5 new, all exercising this change), 0 failed
  • nox -s coverage_report — 96.9% (unchanged from the prior report; no new uncovered lines introduced)
  • nox -s integration_tests — 365 Robot tests passed (was 364), 0 failed
  • nox -s dead_code / nox -s security_scan — clean

Ready for another look whenever convenient.

@hurui200320 Thanks for the review — all three Minor items and the Nit are addressed in `cb48550` (force-pushed to this branch). Details below, checked against `docs/index.md` §4.4/§4.5, ADR-2030 D-9/D-10, and issue #140's acceptance criteria before applying. ### Minor 1 — Missing Behave coverage for the post-loop stuck-model synthesis dispatch site **Done.** Added a scenario ("A post-loop stuck-model synthesis tool dispatch also honors an inherited tools_max_timeout ceiling") in `features/llm_agent_tool_loop.feature` that drives the loop to the third `_build_ephemeral_tool_config()` call site (`tool_config_s`, the post-loop synthesis dispatch) with a `shell` call carrying a `timeout` above the built-in default but within an inherited `tools_max_timeout: 300`, and asserts it succeeds. This now gives Behave coverage across all three dispatch sites named in #140's acceptance criteria (normal round — already covered; budget-exhaustion synthesis — already covered; post-loop synthesis — new). ### Minor 2 — Invalid ceiling test only exercised tools_max_timeout: 0 **Done.** Added three scenarios alongside the existing `tools_max_timeout: 0` case: - negative `tools_max_timeout` (`-5`) - non-numeric `tools_max_timeout` (`not_a_number`) - non-finite `tools_max_timeout` (`nan`) - invalid `shell_max_timeout` (`-5`, with a valid `tools_max_timeout`) All four assert `AgentCreationError` at ephemeral-agent construction, matching #140's "non-numeric, zero, negative, or non-finite... for either tools_max_timeout or shell_max_timeout" criterion. (Implementation note: the negative cases reuse the existing integer-typed Given steps rather than new step definitions, since `-5` is representable there directly — no behavioral difference, just avoided adding redundant step functions.) ### Minor 3 — Robot integration tests do not exercise inherited shell_max_timeout **Done.** Added `LLM Agent's shell_max_timeout Raises The Shell-Only Ceiling For A Dispatched Shell Call (Ticket 140)` to `robot/llm_tool_calling.robot`, using the `shell_max_timeout` parameter already wired into `Create Executor With Shell Tool Calling Agent` (`robot/ToolCallingTestLib.py`). It configures `tools_max_timeout=2`/`shell_max_timeout=100` and issues a per-call `timeout=50` (above the generic ceiling, below the shell-only one) — exactly the case you suggested. ### Nit — Hardcoded fallback values mirror ToolAgent defaults **Done.** Extracted `DEFAULT_TOOLS_MAX_TIMEOUT = 120.0` and `DEFAULT_SHELL_MAX_TIMEOUT = None` as module-level constants in `src/cleveractors/agents/tool.py`, and both `ToolAgent.__init__` and `LLMAgent._build_ephemeral_tool_config()` now read from them instead of each hardcoding the literals independently. This closes the exact drift risk you flagged — the two call sites can no longer diverge silently if the default ever changes. ### Not changed Nothing from your review was skipped or rejected — all four items were verified as correct against the spec/ADR/issue before being applied, and all are now in the diff. ### Verification - `nox -s lint` / `format --check` — green - `nox -s typecheck` — 0 errors - `nox -s unit_tests` — 3094 scenarios passed (5 new, all exercising this change), 0 failed - `nox -s coverage_report` — 96.9% (unchanged from the prior report; no new uncovered lines introduced) - `nox -s integration_tests` — 365 Robot tests passed (was 364), 0 failed - `nox -s dead_code` / `nox -s security_scan` — clean Ready for another look whenever convenient.
CoreRasurae force-pushed feature/m1-inherit-tool-timeout-ceilings from cb48550f16
Some checks failed
CI / lint (pull_request) Successful in 1m58s
CI / typecheck (pull_request) Successful in 1m59s
CI / unit_tests (pull_request) Has started running
CI / integration_tests (pull_request) Has started running
CI / security (pull_request) Successful in 1m0s
CI / benchmark (pull_request) Has started running
CI / build (pull_request) Successful in 38s
CI / coverage (pull_request) Has been cancelled
CI / status-check (pull_request) Has been cancelled
CI / quality (pull_request) Has been cancelled
to d1f17c0b20
Some checks failed
CI / lint (pull_request) Successful in 33s
CI / benchmark (pull_request) Has started running
CI / security (pull_request) Successful in 1m3s
CI / build (pull_request) Successful in 44s
CI / typecheck (pull_request) Successful in 3m6s
CI / quality (pull_request) Successful in 2m26s
CI / integration_tests (pull_request) Successful in 2m15s
CI / unit_tests (pull_request) Successful in 6m21s
CI / coverage (pull_request) Successful in 4m59s
CI / status-check (pull_request) Successful in 3s
CI / typecheck (push) Successful in 1m2s
CI / lint (push) Successful in 1m17s
CI / security (push) Successful in 1m5s
CI / build (push) Successful in 1m31s
CI / quality (push) Successful in 1m41s
CI / integration_tests (push) Successful in 4m28s
CI / unit_tests (push) Successful in 5m36s
CI / coverage (push) Failing after 13m46s
CI / benchmark (push) Failing after 19m15s
CI / status-check (push) Has been cancelled
2026-08-24 10:44:04 +00:00
Compare
CoreRasurae deleted branch feature/m1-inherit-tool-timeout-ceilings 2026-08-24 10:58:37 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
cleveragents/cleveractors-core!141
No description provided.