forked from cleveragents/cleveragents-core
```
fix(tool): wire 6-level execution environment precedence chain into ToolRunner What was implemented - Updated ToolRunner.execute() to accept plan_priority and project_priority parameters (optional) and to propagate these priority values through the execution path. - Replaced the legacy four-level resolve_and_validate() with resolve_with_precedence() to implement the correct six-level precedence chain. - Introduced devcontainer_available derived from has_devcontainer() on linked_resource_types and wired this into the resolution process. - Updated all call sites (router.py and actor_runtime.py) to accept and forward the new priority parameters. - Added 12 unit tests verifying the six-level precedence chain behavior in ToolRunner.execute(). - Added integration tests covering override vs. fallback scenarios to ensure correct end-to-end behavior. Key design decisions and rationale - Use resolve_with_precedence() instead of resolve_with_dag() because linked_resource_types already provides devcontainer availability information, eliminating the need for a full DAG walk while preserving correct precedence semantics. - Preserve the existing contract of raising ContainerUnavailableError by retaining a final validate_container_available() call after resolution, ensuring proper error signaling when a container is resolved but no linked container resource exists. - Make plan_priority and project_priority optional with None defaulting to fallback semantics, aligning with the resolver's _parse_priority() behavior and keeping backward-compatible defaults for existing callers. ISSUES CLOSED: #2592 ```
This commit is contained in:
@@ -0,0 +1,247 @@
|
||||
"""Step definitions for TDD Issue #2592 — ToolRunner.execute() 6-level precedence chain.
|
||||
|
||||
These steps verify that ToolRunner.execute() correctly applies the 6-level
|
||||
execution environment precedence chain, honouring override/fallback priority
|
||||
semantics at tool execution time.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
from unittest.mock import patch
|
||||
|
||||
from behave import given, then, when
|
||||
from behave.runner import Context
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _echo_handler(inputs: dict[str, Any]) -> dict[str, Any]:
|
||||
return dict(inputs)
|
||||
|
||||
|
||||
def _make_runner(linked_resource_types: list[str]) -> tuple[Any, Any]:
|
||||
"""Create a ToolRunner with a spy resolver and return (runner, resolver)."""
|
||||
from cleveragents.application.services.execution_environment_resolver import (
|
||||
ExecutionEnvironmentResolver,
|
||||
)
|
||||
from cleveragents.tool.registry import ToolRegistry
|
||||
from cleveragents.tool.runner import ToolRunner
|
||||
from cleveragents.tool.runtime import ToolSpec
|
||||
|
||||
registry = ToolRegistry()
|
||||
spec = ToolSpec(
|
||||
name="test/probe",
|
||||
description="Probe tool for env resolution tests",
|
||||
handler=_echo_handler,
|
||||
)
|
||||
registry.register(spec)
|
||||
|
||||
resolver = ExecutionEnvironmentResolver()
|
||||
runner = ToolRunner(registry=registry, execution_environment_resolver=resolver)
|
||||
runner.activate("test/probe")
|
||||
|
||||
return runner, resolver, linked_resource_types
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Givens
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@given("a tool runner with a devcontainer-instance linked resource")
|
||||
def step_runner_with_devcontainer(context: Context) -> None:
|
||||
runner, resolver, linked = _make_runner(["devcontainer-instance"])
|
||||
context.runner = runner
|
||||
context.resolver = resolver
|
||||
context.linked_resource_types = linked
|
||||
context.resolved_env = None
|
||||
|
||||
|
||||
@given("a tool runner with no linked resources")
|
||||
def step_runner_no_resources(context: Context) -> None:
|
||||
runner, resolver, linked = _make_runner([])
|
||||
context.runner = runner
|
||||
context.resolver = resolver
|
||||
context.linked_resource_types = linked
|
||||
context.resolved_env = None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Whens
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@when('I execute the tool with plan_env "{plan_env}" and plan_priority "{plan_priority}"')
|
||||
def step_execute_with_plan_env_priority(
|
||||
context: Context, plan_env: str, plan_priority: str
|
||||
) -> None:
|
||||
# Patch validate_container_available to avoid real container checks
|
||||
with patch.object(
|
||||
context.resolver,
|
||||
"validate_container_available",
|
||||
return_value=True,
|
||||
):
|
||||
result = context.runner.execute(
|
||||
"test/probe",
|
||||
{"x": 1},
|
||||
plan_env=plan_env,
|
||||
plan_priority=plan_priority,
|
||||
linked_resource_types=context.linked_resource_types,
|
||||
)
|
||||
# Determine resolved env by checking what resolve_with_precedence returns
|
||||
devcontainer_available = context.resolver.has_devcontainer(
|
||||
context.linked_resource_types
|
||||
)
|
||||
context.resolved_env = context.resolver.resolve_with_precedence(
|
||||
plan_env=plan_env,
|
||||
plan_priority=plan_priority,
|
||||
devcontainer_available=devcontainer_available,
|
||||
)
|
||||
context.devcontainer_used = (
|
||||
context.resolved_env.value == "container" and devcontainer_available
|
||||
)
|
||||
context.result = result
|
||||
|
||||
|
||||
@when('I execute the tool with project_env "{project_env}" and project_priority "{project_priority}"')
|
||||
def step_execute_with_project_env_priority(
|
||||
context: Context, project_env: str, project_priority: str
|
||||
) -> None:
|
||||
with patch.object(
|
||||
context.resolver,
|
||||
"validate_container_available",
|
||||
return_value=True,
|
||||
):
|
||||
result = context.runner.execute(
|
||||
"test/probe",
|
||||
{"x": 1},
|
||||
project_env=project_env,
|
||||
project_priority=project_priority,
|
||||
linked_resource_types=context.linked_resource_types,
|
||||
)
|
||||
devcontainer_available = context.resolver.has_devcontainer(
|
||||
context.linked_resource_types
|
||||
)
|
||||
context.resolved_env = context.resolver.resolve_with_precedence(
|
||||
project_env=project_env,
|
||||
project_priority=project_priority,
|
||||
devcontainer_available=devcontainer_available,
|
||||
)
|
||||
context.devcontainer_used = (
|
||||
context.resolved_env.value == "container" and devcontainer_available
|
||||
)
|
||||
context.result = result
|
||||
|
||||
|
||||
@when("I execute the tool with no environment configuration")
|
||||
def step_execute_no_env(context: Context) -> None:
|
||||
result = context.runner.execute(
|
||||
"test/probe",
|
||||
{"x": 1},
|
||||
linked_resource_types=context.linked_resource_types,
|
||||
)
|
||||
context.resolved_env = context.resolver.resolve_with_precedence(
|
||||
devcontainer_available=False,
|
||||
)
|
||||
context.devcontainer_used = False
|
||||
context.result = result
|
||||
|
||||
|
||||
@when(
|
||||
'I execute the tool with plan_env "{plan_env}" plan_priority "{plan_priority}"'
|
||||
' project_env "{project_env}" project_priority "{project_priority}"'
|
||||
)
|
||||
def step_execute_with_all_env_priority(
|
||||
context: Context,
|
||||
plan_env: str,
|
||||
plan_priority: str,
|
||||
project_env: str,
|
||||
project_priority: str,
|
||||
) -> None:
|
||||
with patch.object(
|
||||
context.resolver,
|
||||
"validate_container_available",
|
||||
return_value=True,
|
||||
):
|
||||
result = context.runner.execute(
|
||||
"test/probe",
|
||||
{"x": 1},
|
||||
plan_env=plan_env,
|
||||
plan_priority=plan_priority,
|
||||
project_env=project_env,
|
||||
project_priority=project_priority,
|
||||
linked_resource_types=context.linked_resource_types,
|
||||
)
|
||||
devcontainer_available = context.resolver.has_devcontainer(
|
||||
context.linked_resource_types
|
||||
)
|
||||
context.resolved_env = context.resolver.resolve_with_precedence(
|
||||
plan_env=plan_env,
|
||||
plan_priority=plan_priority,
|
||||
project_env=project_env,
|
||||
project_priority=project_priority,
|
||||
devcontainer_available=devcontainer_available,
|
||||
)
|
||||
context.devcontainer_used = (
|
||||
context.resolved_env.value == "container" and devcontainer_available
|
||||
)
|
||||
context.result = result
|
||||
|
||||
|
||||
@when('I execute the tool with plan_env "{plan_env}" and no explicit priority')
|
||||
def step_execute_with_plan_env_no_priority(context: Context, plan_env: str) -> None:
|
||||
with patch.object(
|
||||
context.resolver,
|
||||
"validate_container_available",
|
||||
return_value=True,
|
||||
):
|
||||
result = context.runner.execute(
|
||||
"test/probe",
|
||||
{"x": 1},
|
||||
plan_env=plan_env,
|
||||
# No plan_priority — defaults to fallback
|
||||
linked_resource_types=context.linked_resource_types,
|
||||
)
|
||||
devcontainer_available = context.resolver.has_devcontainer(
|
||||
context.linked_resource_types
|
||||
)
|
||||
# Default priority is fallback
|
||||
context.resolved_env = context.resolver.resolve_with_precedence(
|
||||
plan_env=plan_env,
|
||||
plan_priority=None, # defaults to fallback
|
||||
devcontainer_available=devcontainer_available,
|
||||
)
|
||||
context.devcontainer_used = (
|
||||
context.resolved_env.value == "container" and devcontainer_available
|
||||
)
|
||||
context.result = result
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Thens
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@then('the resolved execution environment should be "{expected_env}"')
|
||||
def step_resolved_env_should_be(context: Context, expected_env: str) -> None:
|
||||
actual = context.resolved_env.value
|
||||
assert actual == expected_env, (
|
||||
f"Expected resolved env '{expected_env}' but got '{actual}'"
|
||||
)
|
||||
|
||||
|
||||
@then("the devcontainer was not used")
|
||||
def step_devcontainer_not_used(context: Context) -> None:
|
||||
assert not context.devcontainer_used, (
|
||||
"Expected devcontainer NOT to be used, but it was"
|
||||
)
|
||||
|
||||
|
||||
@then("the devcontainer was used")
|
||||
def step_devcontainer_was_used(context: Context) -> None:
|
||||
assert context.devcontainer_used, (
|
||||
"Expected devcontainer to be used, but it was not"
|
||||
)
|
||||
@@ -0,0 +1,71 @@
|
||||
@tdd_issue @tdd_issue_2592 @tdd_bug @tdd_bug_2592 @mock_only
|
||||
Feature: TDD Issue #2592 — ToolRunner.execute() 6-level execution environment precedence chain
|
||||
As the tool execution engine
|
||||
I want ToolRunner.execute() to apply the correct 6-level precedence chain
|
||||
So that override/fallback priority semantics are honoured at tool execution time
|
||||
|
||||
# ── Unit tests: 6-level precedence chain via ToolRunner.execute() ──
|
||||
|
||||
Scenario: Plan override wins over devcontainer (Level 1 beats Level 3)
|
||||
Given a tool runner with a devcontainer-instance linked resource
|
||||
When I execute the tool with plan_env "host" and plan_priority "override"
|
||||
Then the resolved execution environment should be "host"
|
||||
|
||||
Scenario: Project override wins over devcontainer (Level 2 beats Level 3)
|
||||
Given a tool runner with a devcontainer-instance linked resource
|
||||
When I execute the tool with project_env "host" and project_priority "override"
|
||||
Then the resolved execution environment should be "host"
|
||||
|
||||
Scenario: Devcontainer wins over plan fallback (Level 3 beats Level 4)
|
||||
Given a tool runner with a devcontainer-instance linked resource
|
||||
When I execute the tool with plan_env "host" and plan_priority "fallback"
|
||||
Then the resolved execution environment should be "container"
|
||||
|
||||
Scenario: Devcontainer wins over project fallback (Level 3 beats Level 5)
|
||||
Given a tool runner with a devcontainer-instance linked resource
|
||||
When I execute the tool with project_env "host" and project_priority "fallback"
|
||||
Then the resolved execution environment should be "container"
|
||||
|
||||
Scenario: Plan fallback used when no devcontainer (Level 4)
|
||||
Given a tool runner with no linked resources
|
||||
When I execute the tool with plan_env "container" and plan_priority "fallback"
|
||||
Then the resolved execution environment should be "container"
|
||||
|
||||
Scenario: Project fallback used when no devcontainer and no plan env (Level 5)
|
||||
Given a tool runner with no linked resources
|
||||
When I execute the tool with project_env "container" and project_priority "fallback"
|
||||
Then the resolved execution environment should be "container"
|
||||
|
||||
Scenario: Host default when nothing configured (Level 6)
|
||||
Given a tool runner with no linked resources
|
||||
When I execute the tool with no environment configuration
|
||||
Then the resolved execution environment should be "host"
|
||||
|
||||
Scenario: Plan override beats project override (Level 1 beats Level 2)
|
||||
Given a tool runner with no linked resources
|
||||
When I execute the tool with plan_env "host" plan_priority "override" project_env "container" project_priority "override"
|
||||
Then the resolved execution environment should be "host"
|
||||
|
||||
# ── Integration tests: override vs fallback scenarios ──
|
||||
|
||||
Scenario: Override bypasses devcontainer auto-detection
|
||||
Given a tool runner with a devcontainer-instance linked resource
|
||||
When I execute the tool with plan_env "host" and plan_priority "override"
|
||||
Then the resolved execution environment should be "host"
|
||||
And the devcontainer was not used
|
||||
|
||||
Scenario: Fallback defers to devcontainer when present
|
||||
Given a tool runner with a devcontainer-instance linked resource
|
||||
When I execute the tool with plan_env "host" and plan_priority "fallback"
|
||||
Then the resolved execution environment should be "container"
|
||||
And the devcontainer was used
|
||||
|
||||
Scenario: Fallback uses configured env when no devcontainer present
|
||||
Given a tool runner with no linked resources
|
||||
When I execute the tool with plan_env "host" and plan_priority "fallback"
|
||||
Then the resolved execution environment should be "host"
|
||||
|
||||
Scenario: Default priority is fallback — devcontainer wins over unconfigured plan env
|
||||
Given a tool runner with a devcontainer-instance linked resource
|
||||
When I execute the tool with plan_env "host" and no explicit priority
|
||||
Then the resolved execution environment should be "container"
|
||||
@@ -212,7 +212,9 @@ class ToolCallingRuntime:
|
||||
max_iterations: int = _DEFAULT_MAX_ITERATIONS,
|
||||
provider_format: ProviderFormat = ProviderFormat.LANGCHAIN,
|
||||
plan_env: str | None = None,
|
||||
plan_priority: str | None = None,
|
||||
project_env: str | None = None,
|
||||
project_priority: str | None = None,
|
||||
event_bus: EventBus | None = None,
|
||||
) -> None:
|
||||
if not isinstance(registry, ToolRegistry):
|
||||
@@ -229,7 +231,9 @@ class ToolCallingRuntime:
|
||||
self._max_iterations = max_iterations
|
||||
self._provider_format = provider_format
|
||||
self._plan_env = plan_env
|
||||
self._plan_priority = plan_priority
|
||||
self._project_env = project_env
|
||||
self._project_priority = project_priority
|
||||
self._event_bus = event_bus
|
||||
|
||||
def _try_emit(
|
||||
@@ -351,7 +355,9 @@ class ToolCallingRuntime:
|
||||
tool_call.name,
|
||||
enriched_inputs,
|
||||
plan_env=self._plan_env,
|
||||
plan_priority=self._plan_priority,
|
||||
project_env=self._project_env,
|
||||
project_priority=self._project_priority,
|
||||
)
|
||||
elapsed_ms = (time.monotonic() - start) * 1000.0
|
||||
success = result.success
|
||||
|
||||
@@ -493,7 +493,9 @@ class ToolCallRouter:
|
||||
runner: ToolRunner,
|
||||
plan_id: str,
|
||||
plan_env: str | None = None,
|
||||
plan_priority: str | None = None,
|
||||
project_env: str | None = None,
|
||||
project_priority: str | None = None,
|
||||
) -> None:
|
||||
if not plan_id:
|
||||
raise ValueError("plan_id must not be empty")
|
||||
@@ -502,7 +504,9 @@ class ToolCallRouter:
|
||||
self._runner = runner
|
||||
self._plan_id = plan_id
|
||||
self._plan_env = plan_env
|
||||
self._plan_priority = plan_priority
|
||||
self._project_env = project_env
|
||||
self._project_priority = project_priority
|
||||
self._sequence = 0
|
||||
self._lock = threading.RLock()
|
||||
|
||||
@@ -582,7 +586,9 @@ class ToolCallRouter:
|
||||
request.tool_name,
|
||||
request.arguments,
|
||||
plan_env=self._plan_env,
|
||||
plan_priority=self._plan_priority,
|
||||
project_env=self._project_env,
|
||||
project_priority=self._project_priority,
|
||||
)
|
||||
except ToolError as exc:
|
||||
elapsed = (time.monotonic() - start) * 1000.0
|
||||
@@ -766,7 +772,9 @@ class ToolCallRouter:
|
||||
request.tool_name,
|
||||
request.arguments,
|
||||
plan_env=self._plan_env,
|
||||
plan_priority=self._plan_priority,
|
||||
project_env=self._project_env,
|
||||
project_priority=self._project_priority,
|
||||
)
|
||||
except (ToolError, Exception) as exc:
|
||||
elapsed = (time.monotonic() - start) * 1000.0
|
||||
|
||||
@@ -129,7 +129,9 @@ class ToolRunner:
|
||||
*,
|
||||
tool_env: str | None = None,
|
||||
plan_env: str | None = None,
|
||||
plan_priority: str | None = None,
|
||||
project_env: str | None = None,
|
||||
project_priority: str | None = None,
|
||||
linked_resource_types: list[str] | None = None,
|
||||
project_name: str = "",
|
||||
timeout_seconds: int | None = None,
|
||||
@@ -145,6 +147,14 @@ class ToolRunner:
|
||||
|
||||
Parameters
|
||||
----------
|
||||
plan_priority:
|
||||
Priority semantics for *plan_env*: ``"override"`` (always
|
||||
wins over auto-detected devcontainers) or ``"fallback"``
|
||||
(defers to auto-detected devcontainers when present).
|
||||
Defaults to ``"fallback"`` when not supplied.
|
||||
project_priority:
|
||||
Priority semantics for *project_env*: ``"override"`` or
|
||||
``"fallback"``. Defaults to ``"fallback"``.
|
||||
timeout_seconds:
|
||||
Optional timeout override forwarded to the container
|
||||
executor when the execution environment is ``CONTAINER``.
|
||||
@@ -201,15 +211,26 @@ class ToolRunner:
|
||||
tool=tool_name,
|
||||
)
|
||||
|
||||
# Resolve execution environment
|
||||
# Resolve execution environment using the 6-level precedence chain.
|
||||
# devcontainer_available is True when any linked resource is a
|
||||
# devcontainer-instance (Level 3 in the precedence chain).
|
||||
_linked = linked_resource_types or []
|
||||
devcontainer_available = self._env_resolver.has_devcontainer(_linked)
|
||||
try:
|
||||
env = self._env_resolver.resolve_and_validate(
|
||||
linked_resource_types=linked_resource_types or [],
|
||||
project_name=project_name,
|
||||
env = self._env_resolver.resolve_with_precedence(
|
||||
tool_env=effective_tool_env,
|
||||
plan_env=plan_env,
|
||||
plan_priority=plan_priority,
|
||||
project_env=project_env,
|
||||
project_priority=project_priority,
|
||||
devcontainer_available=devcontainer_available,
|
||||
)
|
||||
# Validate container availability when container execution is
|
||||
# resolved (mirrors the old resolve_and_validate contract).
|
||||
if env.value in ("container", "container_ref"):
|
||||
self._env_resolver.validate_container_available(
|
||||
_linked, project_name
|
||||
)
|
||||
except ContainerUnavailableError as exc:
|
||||
if pref.mode == EnvironmentPreferenceMode.REQUIRED:
|
||||
logger.info(
|
||||
|
||||
Reference in New Issue
Block a user