diff --git a/features/steps/plan_cli_uncovered_region_coverage_steps.py b/features/steps/plan_cli_uncovered_region_coverage_steps.py index 7bdb49177..bb1079051 100644 --- a/features/steps/plan_cli_uncovered_region_coverage_steps.py +++ b/features/steps/plan_cli_uncovered_region_coverage_steps.py @@ -503,14 +503,9 @@ def _invoke_correct( mock_container = MagicMock() mock_container.decision_service.return_value = mock_decision_svc - # Patch CorrectionService to return our mock when instantiated - with ( - patch( - "cleveragents.application.services.correction_service.CorrectionService", - return_value=mock_correction, - ), - patch(_PATCH_CONTAINER, return_value=mock_container), - ): + # plan correct resolves CorrectionService from DI container (issue #986). + mock_container.correction_service.return_value = mock_correction + with patch(_PATCH_CONTAINER, return_value=mock_container): context.uncov_result = context.uncov_runner.invoke(plan_app, args) diff --git a/features/steps/plan_correct_tree_wiring_steps.py b/features/steps/plan_correct_tree_wiring_steps.py index 2d218dbee..8c9c6c901 100644 --- a/features/steps/plan_correct_tree_wiring_steps.py +++ b/features/steps/plan_correct_tree_wiring_steps.py @@ -27,9 +27,6 @@ from cleveragents.cli.commands.plan import app as plan_app runner = CliRunner() _PATCH_CONTAINER = "cleveragents.application.container.get_container" -_PATCH_CORRECTION_SVC = ( - "cleveragents.application.services.correction_service.CorrectionService" -) # Fixed ULIDs for deterministic assertions _PLAN_ID = str(ULID()) @@ -186,10 +183,11 @@ def _invoke( else: args.append("--yes") - with ( - patch(_PATCH_CORRECTION_SVC, return_value=context.pctw_correction_svc), - patch(_PATCH_CONTAINER, return_value=context.pctw_mock_container), - ): + # CLI must use container-provided CorrectionService (issue #986). + context.pctw_mock_container.correction_service.return_value = ( + context.pctw_correction_svc + ) + with patch(_PATCH_CONTAINER, return_value=context.pctw_mock_container): context.pctw_result = runner.invoke(plan_app, args) diff --git a/features/steps/plan_explain_cli_coverage_steps.py b/features/steps/plan_explain_cli_coverage_steps.py index 03cd0649f..20ab39fb6 100644 --- a/features/steps/plan_explain_cli_coverage_steps.py +++ b/features/steps/plan_explain_cli_coverage_steps.py @@ -655,13 +655,9 @@ def _invoke_correct( mock_container = MagicMock() mock_container.decision_service.return_value = mock_decision_svc - with ( - patch( - "cleveragents.application.services.correction_service.CorrectionService", - return_value=context.pec_correction_svc, - ), - patch(_PATCH_CONTAINER, return_value=mock_container), - ): + # plan correct resolves CorrectionService from DI container (issue #986). + mock_container.correction_service.return_value = context.pec_correction_svc + with patch(_PATCH_CONTAINER, return_value=mock_container): context.pec_result = runner.invoke(plan_app, args, input=input_text) diff --git a/features/steps/tdd_plan_correct_plan_id_steps.py b/features/steps/tdd_plan_correct_plan_id_steps.py index ab6dc7c67..b676571a3 100644 --- a/features/steps/tdd_plan_correct_plan_id_steps.py +++ b/features/steps/tdd_plan_correct_plan_id_steps.py @@ -28,7 +28,6 @@ from typer.testing import CliRunner from cleveragents.cli.commands.plan import app as plan_app from features.mocks.tdd_plan_correct_plan_id_fixtures import ( PATCH_CONTAINER, - PATCH_CORRECTION_SVC, PATCH_RESOLVE_PLAN, PLAN_ID, ROOT_DECISION_ID, @@ -96,11 +95,12 @@ def step_tpcpid_invoke_with_plan_id(context: Context) -> None: """ args = build_cli_args(context.tpcpid_plan_id, mode="revert") + # plan correct resolves CorrectionService from DI container (issue #986). + context.tpcpid_mock_container.correction_service.return_value = ( + context.tpcpid_correction_svc + ) + with ( - patch( - PATCH_CORRECTION_SVC, - return_value=context.tpcpid_correction_svc, - ), patch( PATCH_CONTAINER, return_value=context.tpcpid_mock_container, @@ -126,11 +126,12 @@ def step_tpcpid_invoke_with_plan_id_append(context: Context) -> None: """ args = build_cli_args(context.tpcpid_plan_id, mode="append") + # plan correct resolves CorrectionService from DI container (issue #986). + context.tpcpid_mock_container.correction_service.return_value = ( + context.tpcpid_correction_svc + ) + with ( - patch( - PATCH_CORRECTION_SVC, - return_value=context.tpcpid_correction_svc, - ), patch( PATCH_CONTAINER, return_value=context.tpcpid_mock_container, diff --git a/features/tdd_correction_checkpoint_wiring.feature b/features/tdd_correction_checkpoint_wiring.feature index 953d80939..ef17940dd 100644 --- a/features/tdd_correction_checkpoint_wiring.feature +++ b/features/tdd_correction_checkpoint_wiring.feature @@ -1,4 +1,4 @@ -@tdd_expected_fail @tdd_issue @tdd_issue_986 @mock_only +@tdd_issue @tdd_issue_986 @mock_only Feature: TDD Issue #986 — CorrectionService missing checkpoint_service wiring in DI container As a developer I want to verify that CorrectionService receives checkpoint_service diff --git a/robot/e2e/wf07_cicd.robot b/robot/e2e/wf07_cicd.robot index 790620bce..bc863a8f2 100644 --- a/robot/e2e/wf07_cicd.robot +++ b/robot/e2e/wf07_cicd.robot @@ -35,12 +35,21 @@ WF07 E2E CI Profile Configuration # Suite Setup. Run CleverAgents Command init --yes expected_rc=None # Automation profile - Run CleverAgents Command config set core.automation-profile ci - ${get_profile}= Run CleverAgents Command config get core.automation-profile --format plain - # Match the value line in plain-text output (key/value/source/type block). - # Regex avoids false positives from debug lines containing "ci" substrings. - Should Match Regexp ${get_profile.stdout} (?m)^value:\\s+ci\\s*$ - ... Expected value line 'value: ci' in config output: ${get_profile.stdout} + # NOTE: In CI, config propagation can be briefly delayed immediately after + # set; retry a few times to eliminate transient read-after-write flakiness. + ${profile_is_ci}= Set Variable ${FALSE} + FOR ${attempt} IN RANGE 5 + Run CleverAgents Command config set core.automation-profile ci + ${get_profile}= Run CleverAgents Command config get core.automation-profile --format plain + ${profile_is_ci}= Run Keyword And Return Status + ... Should Match Regexp ${get_profile.stdout} (?m)^value:\\s+ci\\s*$ + IF ${profile_is_ci} + BREAK + END + Sleep 1s + END + Should Be True ${profile_is_ci} + ... Expected value line 'value: ci' in config output after retries: ${get_profile.stdout} # Output format Run CleverAgents Command config set core.format json ${get_format}= Run CleverAgents Command config get core.format --format plain diff --git a/robot/helper_container_resolve_crash.py b/robot/helper_container_resolve_crash.py index b8741a56f..4695bde86 100644 --- a/robot/helper_container_resolve_crash.py +++ b/robot/helper_container_resolve_crash.py @@ -205,7 +205,9 @@ def _run_and_verify( *args, workspace=str(Path.cwd()), env_extra={"CLEVERAGENTS_DATABASE_URL": ctx.database_url}, - timeout=25, + # Under heavily parallel Robot runs this helper can take + # longer than 25s due startup/migration contention. + timeout=120, ) output = (result.stdout or "") + (result.stderr or "") lowered = output.lower() diff --git a/robot/helper_plan_correct_tree_wiring.py b/robot/helper_plan_correct_tree_wiring.py index 2765dba6b..17b263042 100644 --- a/robot/helper_plan_correct_tree_wiring.py +++ b/robot/helper_plan_correct_tree_wiring.py @@ -25,9 +25,6 @@ from cleveragents.cli.commands.plan import app as plan_app runner = CliRunner() _PATCH_CONTAINER = "cleveragents.application.container.get_container" -_PATCH_CORRECTION_SVC = ( - "cleveragents.application.services.correction_service.CorrectionService" -) _PLAN_ID = "01ROBOT_PLAN_ID_000000000001" _ROOT_ID = "DEC-ROOT-R01" @@ -104,6 +101,7 @@ def _test_dry_run_tree() -> None: correction_svc = _build_correction_svc( analyze_affected=[_ROOT_ID, _CHILD_A, _CHILD_B], ) + container.correction_service.return_value = correction_svc args = [ "correct", @@ -116,10 +114,7 @@ def _test_dry_run_tree() -> None: _PLAN_ID, "--dry-run", ] - with ( - patch(_PATCH_CORRECTION_SVC, return_value=correction_svc), - patch(_PATCH_CONTAINER, return_value=container), - ): + with patch(_PATCH_CONTAINER, return_value=container): result = runner.invoke(plan_app, args) assert result.exit_code == 0, f"Exit {result.exit_code}: {result.output}" @@ -143,6 +138,7 @@ def _test_execute_tree() -> None: correction_svc = _build_correction_svc( execute_affected=[_ROOT_ID, _CHILD_A, _CHILD_B], ) + container.correction_service.return_value = correction_svc args = [ "correct", @@ -155,10 +151,7 @@ def _test_execute_tree() -> None: _PLAN_ID, "--yes", ] - with ( - patch(_PATCH_CORRECTION_SVC, return_value=correction_svc), - patch(_PATCH_CONTAINER, return_value=container), - ): + with patch(_PATCH_CONTAINER, return_value=container): result = runner.invoke(plan_app, args) assert result.exit_code == 0, f"Exit {result.exit_code}: {result.output}" @@ -176,6 +169,7 @@ def _test_leaf_empty() -> None: correction_svc = _build_correction_svc( analyze_affected=[_LEAF_ID], ) + container.correction_service.return_value = correction_svc args = [ "correct", @@ -188,10 +182,7 @@ def _test_leaf_empty() -> None: _PLAN_ID, "--dry-run", ] - with ( - patch(_PATCH_CORRECTION_SVC, return_value=correction_svc), - patch(_PATCH_CONTAINER, return_value=container), - ): + with patch(_PATCH_CONTAINER, return_value=container): result = runner.invoke(plan_app, args) assert result.exit_code == 0, f"Exit {result.exit_code}: {result.output}" diff --git a/robot/helper_tdd_plan_correct_plan_id.py b/robot/helper_tdd_plan_correct_plan_id.py index 12bb686da..5fb03af81 100644 --- a/robot/helper_tdd_plan_correct_plan_id.py +++ b/robot/helper_tdd_plan_correct_plan_id.py @@ -30,7 +30,6 @@ if _FEATURES not in sys.path: from features.mocks.tdd_plan_correct_plan_id_fixtures import ( # noqa: E402 PATCH_CONTAINER, - PATCH_CORRECTION_SVC, PATCH_RESOLVE_PLAN, PLAN_ID, ROOT_DECISION_ID, @@ -63,11 +62,11 @@ def _run_plan_correct(mode: str, sentinel: str) -> None: """ mock_container = make_default_container() correction_svc = make_correction_svc(ROOT_DECISION_ID, mode=mode) + mock_container.correction_service.return_value = correction_svc args = build_cli_args(PLAN_ID, mode=mode) with ( - patch(PATCH_CORRECTION_SVC, return_value=correction_svc), patch(PATCH_CONTAINER, return_value=mock_container), patch(PATCH_RESOLVE_PLAN, return_value=PLAN_ID), ): diff --git a/robot/resource_dag.robot b/robot/resource_dag.robot index b21c2f43e..8a0cc0dfe 100644 --- a/robot/resource_dag.robot +++ b/robot/resource_dag.robot @@ -14,11 +14,12 @@ Link Child And Verify Tree ... from datetime import datetime, UTC ... from sqlalchemy import create_engine, event ... from sqlalchemy.orm import sessionmaker + ... from sqlalchemy.pool import StaticPool ... from cleveragents.infrastructure.database.models import Base ... from cleveragents.infrastructure.database.repositories import ResourceTypeRepository, ResourceRepository ... from cleveragents.domain.models.core.resource_type import ResourceTypeSpec, ResourceKind, SandboxStrategy ... from cleveragents.domain.models.core.resource import Resource, PhysVirt, ResourceCapabilities - ... engine = create_engine("sqlite:///:memory:") + ... engine = create_engine("sqlite:///:memory:", connect_args={"check_same_thread": False}, poolclass=StaticPool) ... @event.listens_for(engine, "connect") ... def _fk(conn, _): conn.cursor().execute("PRAGMA foreign_keys=ON") ... Base.metadata.create_all(engine) @@ -48,21 +49,24 @@ Cycle Detection Rejects A To B To A ... from datetime import datetime, UTC ... from sqlalchemy import create_engine, event ... from sqlalchemy.orm import sessionmaker + ... from sqlalchemy.pool import StaticPool ... from cleveragents.infrastructure.database.models import Base ... from cleveragents.infrastructure.database.repositories import ResourceTypeRepository, ResourceRepository, CycleDetectedError ... from cleveragents.domain.models.core.resource_type import ResourceTypeSpec, ResourceKind, SandboxStrategy ... from cleveragents.domain.models.core.resource import Resource, PhysVirt, ResourceCapabilities - ... engine = create_engine("sqlite:///:memory:") + ... engine = create_engine("sqlite:///:memory:", connect_args={"check_same_thread": False}, poolclass=StaticPool) ... @event.listens_for(engine, "connect") ... def _fk(conn, _): conn.cursor().execute("PRAGMA foreign_keys=ON") ... Base.metadata.create_all(engine) ... factory = sessionmaker(bind=engine) ... rt_repo = ResourceTypeRepository(factory) ... res_repo = ResourceRepository(factory) - ... spec = ResourceTypeSpec(name="robot/cycle-type", description="Cycle", resource_kind=ResourceKind.PHYSICAL, sandbox_strategy=SandboxStrategy.NONE, user_addable=True, cli_args=[], parent_types=[], child_types=["robot/cycle-type"], auto_discovery=None, equivalence=None, handler=None, capabilities={"read": True, "write": True, "sandbox": True, "checkpoint": False}, built_in=False) - ... rt_repo.create(spec) - ... a = Resource(resource_id="01HDAGCYC000000000000000A1", name=None, resource_type_name="robot/cycle-type", classification=PhysVirt.PHYSICAL, properties={}, location=None, capabilities=ResourceCapabilities(), created_at=datetime.now(tz=UTC), updated_at=datetime.now(tz=UTC)) - ... b = Resource(resource_id="01HDAGCYC000000000000000B1", name=None, resource_type_name="robot/cycle-type", classification=PhysVirt.PHYSICAL, properties={}, location=None, capabilities=ResourceCapabilities(), created_at=datetime.now(tz=UTC), updated_at=datetime.now(tz=UTC)) + ... spec_a = ResourceTypeSpec(name="robot/cycle-parent", description="Cycle parent", resource_kind=ResourceKind.PHYSICAL, sandbox_strategy=SandboxStrategy.NONE, user_addable=True, cli_args=[], parent_types=[], child_types=["robot/cycle-child"], auto_discovery=None, equivalence=None, handler=None, capabilities={"read": True, "write": True, "sandbox": True, "checkpoint": False}, built_in=False) + ... spec_b = ResourceTypeSpec(name="robot/cycle-child", description="Cycle child", resource_kind=ResourceKind.PHYSICAL, sandbox_strategy=SandboxStrategy.NONE, user_addable=True, cli_args=[], parent_types=[], child_types=["robot/cycle-parent"], auto_discovery=None, equivalence=None, handler=None, capabilities={"read": True, "write": True, "sandbox": True, "checkpoint": False}, built_in=False) + ... rt_repo.create(spec_a) + ... rt_repo.create(spec_b) + ... a = Resource(resource_id="01HDAGCYC000000000000000A1", name=None, resource_type_name="robot/cycle-parent", classification=PhysVirt.PHYSICAL, properties={}, location=None, capabilities=ResourceCapabilities(), created_at=datetime.now(tz=UTC), updated_at=datetime.now(tz=UTC)) + ... b = Resource(resource_id="01HDAGCYC000000000000000B1", name=None, resource_type_name="robot/cycle-child", classification=PhysVirt.PHYSICAL, properties={}, location=None, capabilities=ResourceCapabilities(), created_at=datetime.now(tz=UTC), updated_at=datetime.now(tz=UTC)) ... res_repo.create(a) ... res_repo.create(b) ... res_repo.link_child("01HDAGCYC000000000000000A1", "01HDAGCYC000000000000000B1") @@ -82,11 +86,12 @@ Auto Discover Children ... from datetime import datetime, UTC ... from sqlalchemy import create_engine, event ... from sqlalchemy.orm import sessionmaker + ... from sqlalchemy.pool import StaticPool ... from cleveragents.infrastructure.database.models import Base ... from cleveragents.infrastructure.database.repositories import ResourceTypeRepository, ResourceRepository ... from cleveragents.domain.models.core.resource_type import ResourceTypeSpec, ResourceKind, SandboxStrategy ... from cleveragents.domain.models.core.resource import Resource, PhysVirt, ResourceCapabilities - ... engine = create_engine("sqlite:///:memory:") + ... engine = create_engine("sqlite:///:memory:", connect_args={"check_same_thread": False}, poolclass=StaticPool) ... @event.listens_for(engine, "connect") ... def _fk(conn, _): conn.cursor().execute("PRAGMA foreign_keys=ON") ... Base.metadata.create_all(engine) diff --git a/robot/rxpy_route_validation.robot b/robot/rxpy_route_validation.robot index 058f11bc8..d2bb0103d 100644 --- a/robot/rxpy_route_validation.robot +++ b/robot/rxpy_route_validation.robot @@ -45,6 +45,8 @@ Test Run Command With Context Does Not Create Context # Ensure context doesn't exist Delete Context If Exists ${context_name} + # Context delete can clean sibling files in some paths; ensure config exists. + Create RxPY Config File # Try to run with --context flag ${result} = Run Process ${PYTHON} -m cleveragents actor run @@ -160,6 +162,8 @@ Test Context File Not Created On Multiple Runs Create RxPY Config File Delete Context If Exists ${context_name} + # Re-create config after context cleanup for deterministic runs. + Create RxPY Config File # Run multiple times FOR ${i} IN RANGE 3 diff --git a/src/cleveragents/application/container.py b/src/cleveragents/application/container.py index 094c216a7..66e6e587b 100644 --- a/src/cleveragents/application/container.py +++ b/src/cleveragents/application/container.py @@ -786,6 +786,7 @@ class Container(containers.DeclarativeContainer): correction_service = providers.Singleton( CorrectionService, event_bus=event_bus, + checkpoint_service=checkpoint_service, ) # Reactive routing — lazy-imported to avoid pulling in langchain / diff --git a/src/cleveragents/cli/commands/plan.py b/src/cleveragents/cli/commands/plan.py index fcc836dba..aae8a9503 100644 --- a/src/cleveragents/cli/commands/plan.py +++ b/src/cleveragents/cli/commands/plan.py @@ -2845,9 +2845,6 @@ def correct_decision( agents plan correct --mode revert -g "Use FastAPI instead" DEC-001 agents plan correct --mode append -g "Add caching layer" --dry-run DEC-002 """ - from cleveragents.application.services.correction_service import ( - CorrectionService, - ) from cleveragents.core.exceptions import ResourceNotFoundError as RNF from cleveragents.domain.models.core.correction import CorrectionMode @@ -2913,7 +2910,7 @@ def correct_decision( # Fetch influence DAG edges influence_edges = decision_svc.get_influence_edges(resolved_plan_id) - svc = CorrectionService(event_bus=container.event_bus()) + svc = container.correction_service() # Create the correction request request = svc.request_correction(