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

Open
CoreRasurae wants to merge 1 commit from feature/m1-inherit-tool-timeout-ceilings into master
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)

🤖 Generated with Claude Code

## 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) 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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.
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
This pull request can be merged automatically.
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin feature/m1-inherit-tool-timeout-ceilings:feature/m1-inherit-tool-timeout-ceilings
git switch feature/m1-inherit-tool-timeout-ceilings

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch master
git merge --no-ff feature/m1-inherit-tool-timeout-ceilings
git switch feature/m1-inherit-tool-timeout-ceilings
git rebase master
git switch master
git merge --ff-only feature/m1-inherit-tool-timeout-ceilings
git switch feature/m1-inherit-tool-timeout-ceilings
git rebase master
git switch master
git merge --no-ff feature/m1-inherit-tool-timeout-ceilings
git switch master
git merge --squash feature/m1-inherit-tool-timeout-ceilings
git switch master
git merge --ff-only feature/m1-inherit-tool-timeout-ceilings
git switch master
git merge feature/m1-inherit-tool-timeout-ceilings
git push origin master
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.