From 2651e15854d0ee966ca3b216ecd5e5bcb3e19aa8 Mon Sep 17 00:00:00 2001 From: Rui Hu Date: Wed, 25 Mar 2026 11:22:10 +0000 Subject: [PATCH] fix(cli): replace non-existent Container.resolve() with named provider calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three plan.py call sites (tree, explain, correct) that originally used container.resolve(DecisionService) were already fixed in master. This commit completes the fix by aligning the Robot Framework test mocks to match the corrected production code: - robot/helper_m3_decision_validation_smoke.py: mock_container.resolve → mock_container.decision_service (plan correct dry-run path) - robot/helper_m4_correction_subplan_smoke.py: container.resolve → container.decision_service (correction subplan path) Both helpers previously passed silently because MagicMock auto-creates any attribute, masking the mismatch between mock setup and actual code. Added spec=Container to both mock containers so that accessing non-existent attributes raises AttributeError, matching real container behavior. Updated comment wording in features/container_resolve_crash.feature to clarify that the bug is fixed and the scenarios serve as permanent regression guards. The @tdd_expected_fail tag was already removed by the TDD branch before this fix branch was created; the permanent @tdd_issue and @tdd_issue_647 tags remain per TDD tag lifecycle rules. Added CHANGELOG.md entry describing the fix. ISSUES CLOSED: #647 --- CHANGELOG.md | 5 +++++ features/container_resolve_crash.feature | 3 +-- robot/helper_m3_decision_validation_smoke.py | 7 ++++--- robot/helper_m4_correction_subplan_smoke.py | 7 ++++--- 4 files changed, 14 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 31e62213f..40cd73c00 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -152,6 +152,11 @@ Framework integration tests cover override acceptance, fallback acceptance, and full persistence round-trip. Tests use `@tdd_expected_fail` until the fix is merged. (#1100) +- Fixed Robot Framework test mocks for ``plan correct`` dry-run and correction + subplan helpers to use ``container.decision_service()`` instead of the + non-existent ``container.resolve()``, matching corrected production code. + Activated regression-guard BDD scenarios for ``plan tree``, ``plan explain``, + and ``plan correct``. (#647) - Added TDD bug-capture tests for bug #1076 — `use_action()` does not propagate `automation_profile` to Plan. Three Behave BDD scenarios (`@tdd_bug @tdd_bug_1076 @tdd_expected_fail`) verify the full precedence diff --git a/features/container_resolve_crash.feature b/features/container_resolve_crash.feature index d4b8e3fef..5d8055625 100644 --- a/features/container_resolve_crash.feature +++ b/features/container_resolve_crash.feature @@ -7,8 +7,7 @@ Feature: Container.resolve() crash in plan tree/explain/correct commands this class of bug because they mock get_container() with MagicMock, which auto-creates any attribute. - NOTE: Bug #647 appears fixed; these scenarios now run as normal - regression checks for correct command behavior. + NOTE: Bug #647 is fixed; these scenarios serve as permanent regression guards. `plan correct` is intentionally exercised with `--dry-run` in this real-container path so no live LLM execution is triggered. diff --git a/robot/helper_m3_decision_validation_smoke.py b/robot/helper_m3_decision_validation_smoke.py index a282a6d89..2b81fb245 100644 --- a/robot/helper_m3_decision_validation_smoke.py +++ b/robot/helper_m3_decision_validation_smoke.py @@ -21,6 +21,7 @@ if _SRC not in sys.path: from helpers_common import reset_global_state # noqa: E402 from typer.testing import CliRunner # noqa: E402 +from cleveragents.application.container import Container # noqa: E402 from cleveragents.cli.commands.invariant import app as invariant_app # noqa: E402 from cleveragents.cli.commands.plan import app as plan_app # noqa: E402 from cleveragents.cli.commands.validation import app as validation_app # noqa: E402 @@ -275,12 +276,12 @@ def plan_correct_dry_run() -> None: mock_correction_svc.request_correction.return_value = mock_request mock_correction_svc.analyze_impact.return_value = mock_impact - # Mock DecisionService resolved via DI container (issue #606 fix) + # Mock DecisionService provider on DI container (issue #606 / #647 fix) mock_decision_svc = MagicMock() mock_decision_svc.list_decisions.return_value = [] mock_decision_svc.get_influence_edges.return_value = {} - mock_container = MagicMock() - mock_container.resolve.return_value = mock_decision_svc + mock_container = MagicMock(spec=Container) + mock_container.decision_service.return_value = mock_decision_svc with ( patch( diff --git a/robot/helper_m4_correction_subplan_smoke.py b/robot/helper_m4_correction_subplan_smoke.py index ab53f00e6..586ac822d 100644 --- a/robot/helper_m4_correction_subplan_smoke.py +++ b/robot/helper_m4_correction_subplan_smoke.py @@ -21,6 +21,7 @@ from helpers_common import reset_global_state # noqa: E402 from typer.testing import CliRunner # noqa: E402 from ulid import ULID # noqa: E402 +from cleveragents.application.container import Container # noqa: E402 from cleveragents.cli.commands.plan import app as plan_app # noqa: E402 from cleveragents.domain.models.core.correction import ( # noqa: E402 CorrectionImpact, @@ -115,12 +116,12 @@ def _mock_correction_service() -> MagicMock: def _mock_container() -> MagicMock: - """Create a mock DI container that resolves a stub DecisionService.""" + """Create a mock DI container with a stub DecisionService provider.""" mock_decision_svc = MagicMock() mock_decision_svc.list_decisions.return_value = [] mock_decision_svc.get_influence_edges.return_value = {} - container = MagicMock() - container.resolve.return_value = mock_decision_svc + container = MagicMock(spec=Container) + container.decision_service.return_value = mock_decision_svc return container -- 2.52.0