fix(cli): honour project-level execution-env-priority in resolution
CI / build (pull_request) Successful in 15s
CI / helm (pull_request) Successful in 22s
CI / lint (pull_request) Successful in 3m19s
CI / quality (pull_request) Successful in 3m54s
CI / typecheck (pull_request) Successful in 3m57s
CI / security (pull_request) Successful in 4m27s
CI / integration_tests (pull_request) Successful in 7m1s
CI / unit_tests (pull_request) Successful in 7m44s
CI / docker (pull_request) Successful in 1m31s
CI / coverage (pull_request) Successful in 12m18s
CI / e2e_tests (pull_request) Successful in 21m52s
CI / status-check (pull_request) Successful in 1s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 54m34s
CI / build (pull_request) Successful in 15s
CI / helm (pull_request) Successful in 22s
CI / lint (pull_request) Successful in 3m19s
CI / quality (pull_request) Successful in 3m54s
CI / typecheck (pull_request) Successful in 3m57s
CI / security (pull_request) Successful in 4m27s
CI / integration_tests (pull_request) Successful in 7m1s
CI / unit_tests (pull_request) Successful in 7m44s
CI / docker (pull_request) Successful in 1m31s
CI / coverage (pull_request) Successful in 12m18s
CI / e2e_tests (pull_request) Successful in 21m52s
CI / status-check (pull_request) Successful in 1s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 54m34s
Thread plan_env and project_env through the tool execution chain so the ExecutionEnvironmentResolver receives project-level execution environment values stored in ContextConfig.execution_environment. The resolver's precedence logic (tool > plan > project > default) was already correct, but callers never passed project_env — tools always fell through to the global HOST default, ignoring project-level overrides entirely. Changes: - PlanExecutionContext: add plan_env and project_env fields + properties - ToolCallRouter: accept plan_env/project_env in constructor, pass to runner.execute() in route() and route_streaming() - ToolCallingRuntime: accept plan_env/project_env, pass to runner.execute() in the direct-runner fallback path - Update monkey-patched execute stubs in tool_router_steps.py to accept explicit plan_env/project_env keyword arguments - Add 4 BDD scenarios proving plan_env/project_env reach the resolver via ToolCallRouter (project-only, plan-only, both, neither) Dependency: requires PR #1135 to be merged first (adds CLI flag and persistence for project-level execution-env-priority). Closes #1080 ISSUES CLOSED: #1080
This commit is contained in:
+11
-1
@@ -18,6 +18,11 @@
|
||||
plan lifecycle including container commit/push verification on apply.
|
||||
(`robot/e2e/wf18_container_clone.robot`,
|
||||
`src/cleveragents/cli/commands/resource.py`) (#764)
|
||||
- Fixed execution environment resolution to honour project-level override
|
||||
(precedence level 2). Threaded `plan_env` and `project_env` through
|
||||
`ToolCallRouter`, `ToolCallingRuntime`, and `PlanExecutionContext` so
|
||||
the resolver receives project-level execution environment values stored
|
||||
in `ContextConfig.execution_environment`. (#1080)
|
||||
- Added E2E test for Workflow Example 12 — large-scale hierarchical feature
|
||||
implementation (supervised profile). Covers 4-project setup with per-project
|
||||
invariants, spec-compliant action YAML (estimation_actor, invariant_actor,
|
||||
@@ -571,7 +576,12 @@
|
||||
(`@tdd_bug @tdd_bug_1038 @tdd_expected_fail`) verify that the `add`
|
||||
command accepts `--required` and `--informational` flags and that
|
||||
`--required` overrides the YAML config mode. Tests use
|
||||
`@tdd_expected_fail` until the bug fix is merged. (#1102)
|
||||
`@tdd_expected_fail` until the bug fix is merged. (#1102)
|
||||
- Fixed execution environment resolution to honour project-level override
|
||||
(precedence level 2). Threaded `plan_env` and `project_env` through
|
||||
`ToolCallRouter`, `ToolCallingRuntime`, and `PlanExecutionContext` so
|
||||
the resolver receives project-level execution environment values stored
|
||||
in `ContextConfig.execution_environment`. (#1080)
|
||||
- Added BuiltinAdapter class and MCP automatic resource slot creation.
|
||||
BuiltinAdapter wraps register_file_tools/register_git_tools/register_subplan_tool
|
||||
into a unified adapter interface. McpAdapter.infer_resource_slots() analyzes
|
||||
|
||||
@@ -0,0 +1,37 @@
|
||||
Feature: Execution environment project-level override reaches resolver
|
||||
Verifies that project_env threaded through ToolCallRouter
|
||||
is forwarded to the ExecutionEnvironmentResolver inside
|
||||
ToolRunner.execute(), where it is used at precedence level 2.
|
||||
|
||||
Issue #1080: execution environment resolution does not honour
|
||||
project-level override (precedence level 2).
|
||||
|
||||
Scenario: ToolCallRouter forwards project_env to resolver
|
||||
Given ee1080- a tool registry with a registered echo tool
|
||||
And ee1080- a tool runner and resolver spy
|
||||
And ee1080- a ToolCallRouter with project_env "container"
|
||||
When ee1080- I route a tool call through the router
|
||||
Then ee1080- the resolver should have received project_env "container"
|
||||
|
||||
Scenario: ToolCallRouter forwards plan_env to resolver
|
||||
Given ee1080- a tool registry with a registered echo tool
|
||||
And ee1080- a tool runner and resolver spy
|
||||
And ee1080- a ToolCallRouter with plan_env "host"
|
||||
When ee1080- I route a tool call through the router
|
||||
Then ee1080- the resolver should have received plan_env "host"
|
||||
|
||||
Scenario: ToolCallRouter forwards both plan_env and project_env
|
||||
Given ee1080- a tool registry with a registered echo tool
|
||||
And ee1080- a tool runner and resolver spy
|
||||
And ee1080- a ToolCallRouter with both plan_env "host" and project_env "container"
|
||||
When ee1080- I route a tool call through the router
|
||||
Then ee1080- the resolver should have received plan_env "host"
|
||||
And ee1080- the resolver should have received project_env "container"
|
||||
|
||||
Scenario: ToolCallRouter without env params passes None to resolver
|
||||
Given ee1080- a tool registry with a registered echo tool
|
||||
And ee1080- a tool runner and resolver spy
|
||||
And ee1080- a ToolCallRouter without env params
|
||||
When ee1080- I route a tool call through the router
|
||||
Then ee1080- the resolver should have received plan_env "None"
|
||||
And ee1080- the resolver should have received project_env "None"
|
||||
@@ -0,0 +1,129 @@
|
||||
"""Step definitions for exec_env_project_override.feature.
|
||||
|
||||
Verifies that plan_env and project_env are threaded from
|
||||
ToolCallRouter through to the ExecutionEnvironmentResolver
|
||||
inside ToolRunner.execute().
|
||||
|
||||
Issue #1080.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any
|
||||
|
||||
from behave import given, then, when
|
||||
from behave.runner import Context
|
||||
|
||||
from cleveragents.tool.registry import ToolRegistry
|
||||
from cleveragents.tool.router import ToolCallRouter
|
||||
from cleveragents.tool.runner import ToolRunner
|
||||
from cleveragents.tool.runtime import ToolSpec
|
||||
|
||||
__all__: list[str] = []
|
||||
|
||||
|
||||
def _echo_tool(arguments: dict[str, Any]) -> dict[str, Any]:
|
||||
"""Simple echo tool for testing."""
|
||||
return {"echoed": arguments}
|
||||
|
||||
|
||||
@given("ee1080- a tool registry with a registered echo tool")
|
||||
def step_ee1080_registry(context: Context) -> None:
|
||||
context.ee1080_registry = ToolRegistry()
|
||||
spec = ToolSpec(
|
||||
name="test/echo",
|
||||
description="Echo tool for testing",
|
||||
input_schema={"type": "object", "properties": {}},
|
||||
handler=_echo_tool,
|
||||
)
|
||||
context.ee1080_registry.register(spec)
|
||||
|
||||
|
||||
@given("ee1080- a tool runner and resolver spy")
|
||||
def step_ee1080_runner(context: Context) -> None:
|
||||
context.ee1080_runner = ToolRunner(context.ee1080_registry)
|
||||
# Spy on _env_resolver.resolve to capture what it receives
|
||||
context.ee1080_resolve_calls: list[dict[str, Any]] = [] # type: ignore[annotation-unchecked]
|
||||
original_rv = context.ee1080_runner._env_resolver.resolve_and_validate
|
||||
|
||||
def spy_resolve_and_validate(**kwargs: Any) -> tuple[str, str | None]:
|
||||
context.ee1080_resolve_calls.append(
|
||||
{
|
||||
"plan_env": kwargs.get("plan_env"),
|
||||
"project_env": kwargs.get("project_env"),
|
||||
}
|
||||
)
|
||||
return original_rv(**kwargs)
|
||||
|
||||
context.ee1080_runner._env_resolver.resolve_and_validate = spy_resolve_and_validate # type: ignore[method-assign]
|
||||
|
||||
|
||||
@given('ee1080- a ToolCallRouter with project_env "{env}"')
|
||||
def step_ee1080_router_project(context: Context, env: str) -> None:
|
||||
context.ee1080_router = ToolCallRouter(
|
||||
registry=context.ee1080_registry,
|
||||
runner=context.ee1080_runner,
|
||||
plan_id="ee1080-plan",
|
||||
project_env=env,
|
||||
)
|
||||
|
||||
|
||||
@given('ee1080- a ToolCallRouter with plan_env "{env}"')
|
||||
def step_ee1080_router_plan(context: Context, env: str) -> None:
|
||||
context.ee1080_router = ToolCallRouter(
|
||||
registry=context.ee1080_registry,
|
||||
runner=context.ee1080_runner,
|
||||
plan_id="ee1080-plan",
|
||||
plan_env=env,
|
||||
)
|
||||
|
||||
|
||||
@given(
|
||||
'ee1080- a ToolCallRouter with both plan_env "{plan}" and project_env "{project}"'
|
||||
)
|
||||
def step_ee1080_router_both(context: Context, plan: str, project: str) -> None:
|
||||
context.ee1080_router = ToolCallRouter(
|
||||
registry=context.ee1080_registry,
|
||||
runner=context.ee1080_runner,
|
||||
plan_id="ee1080-plan",
|
||||
plan_env=plan,
|
||||
project_env=project,
|
||||
)
|
||||
|
||||
|
||||
@given("ee1080- a ToolCallRouter without env params")
|
||||
def step_ee1080_router_none(context: Context) -> None:
|
||||
context.ee1080_router = ToolCallRouter(
|
||||
registry=context.ee1080_registry,
|
||||
runner=context.ee1080_runner,
|
||||
plan_id="ee1080-plan",
|
||||
)
|
||||
|
||||
|
||||
@when("ee1080- I route a tool call through the router")
|
||||
def step_ee1080_route(context: Context) -> None:
|
||||
payload = {
|
||||
"name": "test/echo",
|
||||
"arguments": {"msg": "hello"},
|
||||
}
|
||||
context.ee1080_result = context.ee1080_router.route(payload)
|
||||
|
||||
|
||||
@then('ee1080- the resolver should have received project_env "{expected}"')
|
||||
def step_ee1080_check_project_env(context: Context, expected: str) -> None:
|
||||
expected_val = None if expected == "None" else expected
|
||||
calls = context.ee1080_resolve_calls
|
||||
assert len(calls) > 0, "Resolver was never called"
|
||||
actual = calls[-1]["project_env"]
|
||||
assert actual == expected_val, (
|
||||
f"Expected project_env={expected_val!r}, got {actual!r}"
|
||||
)
|
||||
|
||||
|
||||
@then('ee1080- the resolver should have received plan_env "{expected}"')
|
||||
def step_ee1080_check_plan_env(context: Context, expected: str) -> None:
|
||||
expected_val = None if expected == "None" else expected
|
||||
calls = context.ee1080_resolve_calls
|
||||
assert len(calls) > 0, "Resolver was never called"
|
||||
actual = calls[-1]["plan_env"]
|
||||
assert actual == expected_val, f"Expected plan_env={expected_val!r}, got {actual!r}"
|
||||
@@ -780,7 +780,14 @@ def step_registry_runtime_error(context: Context) -> None:
|
||||
runner = ToolRunner(registry)
|
||||
|
||||
# Override execute to raise RuntimeError, bypassing ToolRunner's catch
|
||||
def _raising_execute(name: str, arguments: dict[str, Any]) -> ToolResult:
|
||||
def _raising_execute(
|
||||
name: str,
|
||||
arguments: dict[str, Any],
|
||||
*,
|
||||
plan_env: str | None = None,
|
||||
project_env: str | None = None,
|
||||
**_kwargs: Any,
|
||||
) -> ToolResult:
|
||||
raise RuntimeError("boom")
|
||||
|
||||
runner.execute = _raising_execute # type: ignore[assignment]
|
||||
@@ -825,7 +832,14 @@ def step_registry_fail_result(context: Context) -> None:
|
||||
runner = ToolRunner(registry)
|
||||
|
||||
# Override execute to return a ToolResult with success=False
|
||||
def _execute_fail(name: str, arguments: dict[str, Any]) -> ToolResult:
|
||||
def _execute_fail(
|
||||
name: str,
|
||||
arguments: dict[str, Any],
|
||||
*,
|
||||
plan_env: str | None = None,
|
||||
project_env: str | None = None,
|
||||
**_kwargs: Any,
|
||||
) -> ToolResult:
|
||||
return ToolResult(
|
||||
success=False,
|
||||
output={},
|
||||
|
||||
@@ -82,6 +82,8 @@ class PlanExecutionContext:
|
||||
project_resources: dict[str, Any] | None = None,
|
||||
resource_bindings: dict[str, BoundResource] | None = None,
|
||||
changeset_store: ChangeSetStore | None = None,
|
||||
plan_env: str | None = None,
|
||||
project_env: str | None = None,
|
||||
) -> None:
|
||||
if not plan_id:
|
||||
raise ValidationError("plan_id must not be empty")
|
||||
@@ -96,6 +98,8 @@ class PlanExecutionContext:
|
||||
self._resource_bindings: dict[str, BoundResource] = (
|
||||
resource_bindings if resource_bindings is not None else {}
|
||||
)
|
||||
self._plan_env = plan_env
|
||||
self._project_env = project_env
|
||||
self._changeset_store: ChangeSetStore = (
|
||||
changeset_store if changeset_store is not None else InMemoryChangeSetStore()
|
||||
)
|
||||
@@ -139,6 +143,16 @@ class PlanExecutionContext:
|
||||
"""Automation profile name."""
|
||||
return self._automation_profile
|
||||
|
||||
@property
|
||||
def plan_env(self) -> str | None:
|
||||
"""Plan-level execution environment override."""
|
||||
return self._plan_env
|
||||
|
||||
@property
|
||||
def project_env(self) -> str | None:
|
||||
"""Project-level execution environment override."""
|
||||
return self._project_env
|
||||
|
||||
@property
|
||||
def project_resources(self) -> dict[str, Any]:
|
||||
"""Project resources mapping."""
|
||||
|
||||
@@ -206,6 +206,8 @@ class ToolCallingRuntime:
|
||||
router: ToolCallRouter | None = None,
|
||||
max_iterations: int = _DEFAULT_MAX_ITERATIONS,
|
||||
provider_format: ProviderFormat = ProviderFormat.LANGCHAIN,
|
||||
plan_env: str | None = None,
|
||||
project_env: str | None = None,
|
||||
) -> None:
|
||||
if not isinstance(registry, ToolRegistry):
|
||||
raise TypeError("registry must be a ToolRegistry")
|
||||
@@ -220,6 +222,8 @@ class ToolCallingRuntime:
|
||||
self._router = router
|
||||
self._max_iterations = max_iterations
|
||||
self._provider_format = provider_format
|
||||
self._plan_env = plan_env
|
||||
self._project_env = project_env
|
||||
|
||||
# -- Properties -----------------------------------------------------------
|
||||
|
||||
@@ -323,7 +327,12 @@ class ToolCallingRuntime:
|
||||
else:
|
||||
# Execute directly via runner, catching ToolError for not-found
|
||||
try:
|
||||
result = self._runner.execute(tool_call.name, enriched_inputs)
|
||||
result = self._runner.execute(
|
||||
tool_call.name,
|
||||
enriched_inputs,
|
||||
plan_env=self._plan_env,
|
||||
project_env=self._project_env,
|
||||
)
|
||||
elapsed_ms = (time.monotonic() - start) * 1000.0
|
||||
success = result.success
|
||||
output = result.output
|
||||
|
||||
@@ -492,6 +492,8 @@ class ToolCallRouter:
|
||||
registry: ToolRegistry,
|
||||
runner: ToolRunner,
|
||||
plan_id: str,
|
||||
plan_env: str | None = None,
|
||||
project_env: str | None = None,
|
||||
) -> None:
|
||||
if not plan_id:
|
||||
raise ValueError("plan_id must not be empty")
|
||||
@@ -499,6 +501,8 @@ class ToolCallRouter:
|
||||
self._registry = registry
|
||||
self._runner = runner
|
||||
self._plan_id = plan_id
|
||||
self._plan_env = plan_env
|
||||
self._project_env = project_env
|
||||
self._sequence = 0
|
||||
self._lock = threading.RLock()
|
||||
|
||||
@@ -571,10 +575,15 @@ class ToolCallRouter:
|
||||
is_validation=False,
|
||||
)
|
||||
|
||||
# Execute the tool
|
||||
# Execute the tool with execution environment context
|
||||
start = time.monotonic()
|
||||
try:
|
||||
result = self._runner.execute(request.tool_name, request.arguments)
|
||||
result = self._runner.execute(
|
||||
request.tool_name,
|
||||
request.arguments,
|
||||
plan_env=self._plan_env,
|
||||
project_env=self._project_env,
|
||||
)
|
||||
except ToolError as exc:
|
||||
elapsed = (time.monotonic() - start) * 1000.0
|
||||
error_cat = classify_tool_error(str(exc))
|
||||
@@ -751,9 +760,14 @@ class ToolCallRouter:
|
||||
elapsed_ms=0.0,
|
||||
)
|
||||
|
||||
# Execute
|
||||
# Execute with execution environment context
|
||||
try:
|
||||
result = self._runner.execute(request.tool_name, request.arguments)
|
||||
result = self._runner.execute(
|
||||
request.tool_name,
|
||||
request.arguments,
|
||||
plan_env=self._plan_env,
|
||||
project_env=self._project_env,
|
||||
)
|
||||
except (ToolError, Exception) as exc:
|
||||
elapsed = (time.monotonic() - start) * 1000.0
|
||||
error_cat = classify_tool_error(str(exc))
|
||||
|
||||
Reference in New Issue
Block a user