diff --git a/features/decision_phase_gating.feature b/features/decision_phase_gating.feature index 46daaa283..06c90a72d 100644 --- a/features/decision_phase_gating.feature +++ b/features/decision_phase_gating.feature @@ -113,8 +113,8 @@ Feature: Decision type phase-gating at recording time When I try to record a "strategy_choice" decision without explicit phase Then a phase violation error should be raised - Scenario: Phase-gating skipped when plan not found in database - Given a phase-gated decision service with an empty database + Scenario: Phase-gating skipped for ungated persisted phase + Given a phase-gated decision service with a persisted apply plan When I record a "tool_invocation" decision without explicit phase Then the phase-gated decision should be recorded successfully diff --git a/features/steps/decision_phase_gating_steps.py b/features/steps/decision_phase_gating_steps.py index 221c8dd5a..df131504a 100644 --- a/features/steps/decision_phase_gating_steps.py +++ b/features/steps/decision_phase_gating_steps.py @@ -23,6 +23,7 @@ from cleveragents.application.services.phase_gating import ( validate_phase_gating, ) from cleveragents.core.exceptions import DecisionPhaseViolationError, ValidationError +from cleveragents.domain.models.core.action import Action from cleveragents.domain.models.core.decision import ( EXECUTE_TYPES, STRATEGIZE_TYPES, @@ -57,6 +58,7 @@ def step_phase_gated_service(context: Context) -> None: context.pg_error = None context._pg_plan_id = str(ULID()) context._pg_seq = 0 + context._pg_root_decision_id = None # --------------------------------------------------------------------------- @@ -76,22 +78,10 @@ def step_persisted_execute_plan(context: Context) -> None: _setup_persisted_plan(context, PlanPhase.EXECUTE) -@given("a phase-gated decision service with an empty database") -def step_persisted_empty_db(context: Context) -> None: - """Create a DecisionService with a wired UoW but no plans in the DB.""" - fd, db_path = tempfile.mkstemp(suffix=".db", prefix="pg_empty_") - os.close(fd) - context._pg_db_path = db_path - uow = UnitOfWork(f"sqlite:///{db_path}") - uow.init_database() - - context._pg_plan_id = str(ULID()) - context.pg_service = DecisionService(unit_of_work=uow) - context.pg_result = None - context.pg_error = None - context._pg_seq = 0 - - _register_db_cleanup(context, db_path, uow) +@given("a phase-gated decision service with a persisted apply plan") +def step_persisted_apply_plan(context: Context) -> None: + """Create a DecisionService backed by a real database with an ungated plan.""" + _setup_persisted_plan(context, PlanPhase.APPLY) def _setup_persisted_plan(context: Context, phase: PlanPhase) -> None: @@ -104,7 +94,9 @@ def _setup_persisted_plan(context: Context, phase: PlanPhase) -> None: plan_id = str(ULID()) context._pg_plan_id = plan_id + context._pg_root_decision_id = None + action = _phase_test_action() plan = Plan( identity=PlanIdentity(plan_id=plan_id), namespaced_name=NamespacedName(namespace="local", name="pg-test"), @@ -114,6 +106,7 @@ def _setup_persisted_plan(context: Context, phase: PlanPhase) -> None: processing_state=ProcessingState.PROCESSING, ) with uow.transaction() as txn: + txn.actions.create(action) txn.lifecycle_plans.create(plan) context.pg_service = DecisionService(unit_of_work=uow) @@ -138,6 +131,40 @@ def _register_db_cleanup(context: Context, db_path: str, uow: UnitOfWork) -> Non context._cleanup_handlers.append(_cleanup_db) +def _phase_test_action() -> Action: + """Return the minimal action row required by persisted phase-gating plans.""" + return Action( + namespaced_name=NamespacedName(namespace="local", name="pg-test-action"), + description="Phase gating test action", + definition_of_done="Phase gating test done", + strategy_actor="test/strategy", + execution_actor="test/execution", + created_by="phase-gating-test", + ) + + +def _ensure_persisted_parent_decision( + context: Context, + plan_id: str, + decision_type: DecisionType, +) -> str | None: + """Create a persisted root decision before child decisions when FKs apply.""" + if decision_type == DecisionType.PROMPT_DEFINITION: + return None + if not getattr(context.pg_service, "_persisted", False): + return str(ULID()) + if getattr(context, "_pg_root_decision_id", None) is None: + root = context.pg_service.record_decision( + plan_id=plan_id, + decision_type=DecisionType.PROMPT_DEFINITION, + question="Phase-gated root question", + chosen_option="Phase-gated root choice", + plan_phase=PlanPhase.STRATEGIZE, + ) + context._pg_root_decision_id = root.decision_id + return context._pg_root_decision_id + + @given("a phase-gated decision service without persistence") def step_no_persistence_service(context: Context) -> None: context.pg_service = DecisionService() @@ -167,11 +194,8 @@ def step_record_with_phase(context: Context, dtype: str, phase: str) -> None: svc: DecisionService = context.pg_service plan_id = _pg_plan_id(context) - # Determine parent_decision_id: prompt_definition must be root - parent_id = None dt = DecisionType(dtype) - if dt != DecisionType.PROMPT_DEFINITION: - parent_id = str(ULID()) + parent_id = _ensure_persisted_parent_decision(context, plan_id, dt) d = svc.record_decision( plan_id=plan_id, @@ -190,7 +214,11 @@ def step_try_record_with_phase(context: Context, dtype: str, phase: str) -> None svc: DecisionService = context.pg_service plan_id = _pg_plan_id(context) - parent_id = str(ULID()) + parent_id = _ensure_persisted_parent_decision( + context, + plan_id, + DecisionType(dtype), + ) try: svc.record_decision( @@ -211,10 +239,8 @@ def step_record_without_phase(context: Context, dtype: str) -> None: svc: DecisionService = context.pg_service plan_id = _pg_plan_id(context) - parent_id = str(ULID()) dt = DecisionType(dtype) - if dt == DecisionType.PROMPT_DEFINITION: - parent_id = None + parent_id = _ensure_persisted_parent_decision(context, plan_id, dt) try: d = svc.record_decision( @@ -253,10 +279,8 @@ def step_record_with_string_phase(context: Context, dtype: str, phase: str) -> N svc: DecisionService = context.pg_service plan_id = _pg_plan_id(context) - parent_id = None dt = DecisionType(dtype) - if dt != DecisionType.PROMPT_DEFINITION: - parent_id = str(ULID()) + parent_id = _ensure_persisted_parent_decision(context, plan_id, dt) d = svc.record_decision( plan_id=plan_id, @@ -275,10 +299,8 @@ def step_record_with_enum_phase(context: Context, dtype: str, phase: str) -> Non svc: DecisionService = context.pg_service plan_id = _pg_plan_id(context) - parent_id = None dt = DecisionType(dtype) - if dt != DecisionType.PROMPT_DEFINITION: - parent_id = str(ULID()) + parent_id = _ensure_persisted_parent_decision(context, plan_id, dt) d = svc.record_decision( plan_id=plan_id, @@ -367,7 +389,11 @@ def step_phase_map_execute(context: Context, phase: str) -> None: def step_try_record_invalid_phase(context: Context, dtype: str, phase: str) -> None: svc: DecisionService = context.pg_service plan_id = _pg_plan_id(context) - parent_id = str(ULID()) + parent_id = _ensure_persisted_parent_decision( + context, + plan_id, + DecisionType(dtype), + ) try: svc.record_decision( diff --git a/features/steps/decision_recording_steps.py b/features/steps/decision_recording_steps.py index 74f476145..c58045e3f 100644 --- a/features/steps/decision_recording_steps.py +++ b/features/steps/decision_recording_steps.py @@ -24,12 +24,20 @@ from cleveragents.application.services.decision_service import ( SnapshotStore, ) from cleveragents.core.exceptions import ValidationError +from cleveragents.domain.models.core.action import Action from cleveragents.domain.models.core.decision import ( ArtifactRef, ContextSnapshot, Decision, DecisionType, ) +from cleveragents.domain.models.core.plan import ( + NamespacedName, + Plan, + PlanIdentity, + PlanPhase, + ProcessingState, +) def _resolve_plan_id(context: Context, symbolic_id: str) -> str: @@ -43,9 +51,45 @@ def _resolve_plan_id(context: Context, symbolic_id: str) -> str: if symbolic_id not in registry: registry[symbolic_id] = str(ULID()) context._plan_id_registry = registry + _ensure_persisted_plan(context, registry[symbolic_id]) return registry[symbolic_id] +def _dsvc_test_action() -> Action: + """Return the minimal action row required by persisted decision tests.""" + return Action( + namespaced_name=NamespacedName(namespace="local", name="dsvc-test-action"), + description="Decision service persistence test action", + definition_of_done="Decision service persistence test done", + strategy_actor="test/strategy", + execution_actor="test/execution", + created_by="decision-recording-test", + ) + + +def _ensure_persisted_plan(context: Context, plan_id: str) -> None: + """Seed FK prerequisites for persisted decision-service scenarios.""" + uow = getattr(context, "_dsvc_uow", None) + if uow is None: + return + + with uow.transaction() as txn: + action_name = "local/dsvc-test-action" + if txn.actions.get_by_id(action_name) is None: + txn.actions.create(_dsvc_test_action()) + if txn.lifecycle_plans.get(plan_id) is None: + txn.lifecycle_plans.create( + Plan( + identity=PlanIdentity(plan_id=plan_id), + namespaced_name=NamespacedName(namespace="local", name="dsvc-test"), + description="Decision service persistence test plan", + action_name=action_name, + phase=PlanPhase.APPLY, + processing_state=ProcessingState.PROCESSING, + ) + ) + + # --------------------------------------------------------------------------- # Background # --------------------------------------------------------------------------- @@ -67,6 +111,7 @@ def step_decision_service(context: Context) -> None: context.delete_result = None context.explicit_snapshot = None context._plan_id_registry = {} + context._dsvc_uow = None # --------------------------------------------------------------------------- @@ -1042,6 +1087,7 @@ def step_persisted_decision_service(context: Context) -> None: context._dsvc_db_path = db_path uow = UnitOfWork(f"sqlite:///{db_path}") uow.init_database() + context._dsvc_uow = uow context.decision_service = DecisionService(unit_of_work=uow) context.recorded_decisions = [] @@ -1081,6 +1127,7 @@ def step_recreate_service_same_db(context: Context) -> None: db_path = context._dsvc_db_path uow = UnitOfWork(f"sqlite:///{db_path}") uow.init_database() + context._dsvc_uow = uow context.decision_service = DecisionService(unit_of_work=uow) # Preserve _plan_id_registry so symbolic IDs ("P1") still resolve