diff --git a/CHANGELOG.md b/CHANGELOG.md index ff6016f8d..80d968378 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -200,6 +200,14 @@ back to the in-memory cache on database errors or when no Unit of Work is wired. Added `ActionRepository.list_all()` for unfiltered action listing. (#760) +- Added TDD bug-capture test for bug #1080 — execution environment resolution + ignores project-level override. Three Behave BDD scenarios + (`@tdd_bug @tdd_bug_1080 @mock_only`) verify the 6-level precedence chain + defined in §Execution Environment Routing. The critical scenario uses + `@tdd_expected_fail` to confirm the bug: project-level override (level 2) + incorrectly loses to plan-level fallback (level 4). Two regression guard + scenarios verify existing correct behaviour (plan override vs project + override, project override vs host default). (#1101) - 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 diff --git a/features/steps/tdd_exec_env_resolution_precedence_steps.py b/features/steps/tdd_exec_env_resolution_precedence_steps.py new file mode 100644 index 000000000..126086192 --- /dev/null +++ b/features/steps/tdd_exec_env_resolution_precedence_steps.py @@ -0,0 +1,174 @@ +"""Step definitions for TDD bug #1080 — execution env resolution precedence. + +This file captures bug #1080: the ExecutionEnvironmentResolver does not +honour the 6-level precedence chain defined in the spec (§Execution +Environment Routing). The resolver currently uses a flat chain +(tool > plan > project > default) without distinguishing "override" +from "fallback" priority semantics. + +Scenario 1 is tagged ``@tdd_expected_fail`` so CI passes while the bug +is unfixed. Scenarios 2 and 3 are regression guards (no expected-fail +tag) that verify existing correct behaviour is not broken. + +When the fix for #1080 is implemented: + +1. Remove the ``@tdd_expected_fail`` tag from Scenario 1 in the feature file. +2. Update the resolver to implement the 6-level precedence chain with + priority-aware interleaving. +3. All three scenarios should then pass normally. +""" + +from __future__ import annotations + +from behave import given, then, when # type: ignore[import-untyped] +from behave.runner import Context # type: ignore[import-untyped] + +from cleveragents.application.services.execution_environment_resolver import ( + ExecutionEnvironmentResolver, +) + +# Valid priority values accepted by the step definitions. Guards against +# typos in feature files and ensures parity with the spec's two-mode model. +_VALID_PRIORITIES: frozenset[str] = frozenset({"override", "fallback"}) + + +@given("a precedence-aware execution environment resolver") +def step_create_precedence_resolver(context: Context) -> None: + """Create an ExecutionEnvironmentResolver for precedence testing.""" + context.precedence_resolver = ExecutionEnvironmentResolver() + context.precedence_resolved_env = None + + +@when( + 'I resolve precedence with plan env "{plan_env}" at priority ' + '"{plan_priority}" and project env "{project_env}" at priority ' + '"{project_priority}"' +) +def step_resolve_precedence_with_priorities( + context: Context, + plan_env: str, + plan_priority: str, + project_env: str, + project_priority: str, +) -> None: + """Resolve execution environment with the 6-level precedence chain. + + Per the spec, the correct precedence is: + 1. Plan-level with priority: override + 2. Project-level with priority: override + 3. Nearest-ancestor devcontainer + 4. Plan-level with priority: fallback + 5. Project-level with priority: fallback + 6. Host (default) + + The current ``resolver.resolve()`` uses a flat chain that ignores + priority, so this test will produce incorrect results for Scenario 1 + until the bug is fixed. Scenarios 2 and 3 exercise paths that + happen to work correctly with the flat chain but still route through + this step to ensure the resolver sees both competing environments. + """ + # Validate priority values against the spec's two-mode model. + assert plan_priority in _VALID_PRIORITIES, ( + f"Invalid plan_priority '{plan_priority}'; " + f"expected one of {sorted(_VALID_PRIORITIES)}" + ) + assert project_priority in _VALID_PRIORITIES, ( + f"Invalid project_priority '{project_priority}'; " + f"expected one of {sorted(_VALID_PRIORITIES)}" + ) + + resolver: ExecutionEnvironmentResolver = context.precedence_resolver + + # TODO(#1080): The 6-level precedence chain is built manually here + # because the current resolver.resolve() uses a flat priority chain + # that ignores priority semantics. When the bug is fixed, this + # manual chain should be replaced by a single call to the resolver's + # new priority-aware API. + + # Level 1: plan override + if plan_env and plan_priority == "override": + # Pass both plan_env AND project_env so the resolver must choose + # correctly when competing environments are present. + context.precedence_resolved_env = resolver.resolve( + plan_env=plan_env, + project_env=project_env, + ) + return + + # Level 2: project override + if project_env and project_priority == "override": + # BUG: the current resolver.resolve() does not support this + # precedence level. Calling resolve(plan_env=..., project_env=...) + # always returns plan_env first. We pass both plan_env and + # project_env to the resolver and assert the result matches the + # project override. The current flat chain will return plan_env + # instead — triggering the expected assertion failure. + context.precedence_resolved_env = resolver.resolve( + plan_env=plan_env, + project_env=project_env, + ) + return + + # Level 3: devcontainer (not tested in this scenario) + + # Level 4: plan fallback + if plan_env and plan_priority == "fallback": + context.precedence_resolved_env = resolver.resolve(plan_env=plan_env) + return + + # Level 5: project fallback + if project_env and project_priority == "fallback": + context.precedence_resolved_env = resolver.resolve(project_env=project_env) + return + + # Level 6: host default + context.precedence_resolved_env = resolver.resolve() + + +@when( + 'I resolve precedence with no plan env and project env "{project_env}" ' + 'at priority "{project_priority}"' +) +def step_resolve_precedence_project_only( + context: Context, + project_env: str, + project_priority: str, +) -> None: + """Resolve with project-level environment and no plan-level environment. + + When no plan-level environment is set, the project-level environment + should be returned regardless of priority mode. This scenario acts + as a regression guard: the current resolver handles this case + correctly (project_env is returned when plan_env is None), but the + resolution path does not check priority — it works only by accident. + + This scenario ensures the fix correctly handles the project-override + path and doesn't regress the simple case. + """ + # Validate the priority parameter to catch silent discards. + assert project_priority in _VALID_PRIORITIES, ( + f"Invalid project_priority '{project_priority}'; " + f"expected one of {sorted(_VALID_PRIORITIES)}" + ) + + resolver: ExecutionEnvironmentResolver = context.precedence_resolver + context.precedence_resolved_env = resolver.resolve(project_env=project_env) + + +@then('the precedence-resolved environment should be "{expected}"') +def step_check_precedence_result(context: Context, expected: str) -> None: + """Assert the resolved environment matches the expected value. + + For bug #1080, the critical assertion is in the scenario + 'Project-level override beats plan-level fallback': the resolver + is called with plan_env="host" and project_env="container", but + the expected result is "container" (project override at level 2 + beats plan fallback at level 4). The current resolver returns + "host" because it always picks plan_env before project_env. + """ + assert context.precedence_resolved_env is not None, "No environment was resolved" + assert context.precedence_resolved_env.value == expected, ( + f"Expected execution environment '{expected}' but got " + f"'{context.precedence_resolved_env.value}'. " + f"Bug #1080: resolver does not honour 6-level precedence chain." + ) diff --git a/features/tdd_exec_env_resolution_precedence.feature b/features/tdd_exec_env_resolution_precedence.feature new file mode 100644 index 000000000..11e249451 --- /dev/null +++ b/features/tdd_exec_env_resolution_precedence.feature @@ -0,0 +1,47 @@ +@tdd_issue @tdd_issue_1080 @tdd_bug @tdd_bug_1080 @mock_only +Feature: TDD Bug #1080 — execution environment resolution ignores project-level override + As a developer + I want execution environment resolution to honour the 6-level precedence chain + So that project-level overrides (precedence level 2) take effect correctly + + # Bug #1080: The ExecutionEnvironmentResolver uses a flat priority chain + # (tool > plan > project > default) that does not distinguish between + # "override" and "fallback" priority semantics. Per the spec (§Execution + # Environment Routing), the correct 6-level precedence is: + # + # 1. Plan-level with priority: override + # 2. Project-level with priority: override + # 3. Nearest-ancestor devcontainer (auto-detected) + # 4. Plan-level with priority: fallback + # 5. Project-level with priority: fallback + # 6. Host (default) + # + # The critical failure: when a project has execution_environment set with + # priority "override" and a plan has execution_environment set with + # priority "fallback", the plan incorrectly wins because the resolver + # treats all plan-level settings as higher priority than project-level. + # + # This test captures the bug by asserting the CORRECT precedence behavior. + # The @tdd_expected_fail tag on the bug-capturing scenario inverts the + # result: the assertion fails (proving the bug exists) but CI reports it + # as passed. When bug #1080 is fixed, the @tdd_expected_fail tag must + # be removed. + # + # The @mock_only tag restricts this feature to in-process mock execution; + # it does not require any external services or container infrastructure. + + @tdd_expected_fail + Scenario: Bug #1080 - Project-level override beats plan-level fallback (precedence level 2 vs 4) + Given a precedence-aware execution environment resolver + When I resolve precedence with plan env "host" at priority "fallback" and project env "container" at priority "override" + Then the precedence-resolved environment should be "container" + + Scenario: Regression guard - Plan-level override still beats project-level override (precedence level 1 vs 2) + Given a precedence-aware execution environment resolver + When I resolve precedence with plan env "host" at priority "override" and project env "container" at priority "override" + Then the precedence-resolved environment should be "host" + + Scenario: Regression guard - Project-level override beats host default (precedence level 2 vs 6) + Given a precedence-aware execution environment resolver + When I resolve precedence with no plan env and project env "container" at priority "override" + Then the precedence-resolved environment should be "container"