diff --git a/features/steps/plan_lifecycle_service_steps.py b/features/steps/plan_lifecycle_service_steps.py index 16e4e4226..e6745e2f1 100644 --- a/features/steps/plan_lifecycle_service_steps.py +++ b/features/steps/plan_lifecycle_service_steps.py @@ -24,6 +24,7 @@ from cleveragents.domain.models.core.action import ( ) from cleveragents.domain.models.core.plan import ( PlanPhase, + ProcessingState, ProjectLink, ) @@ -468,40 +469,60 @@ def step_create_plan_strategize_complete(context: Context) -> None: def step_create_plan_execute_queued(context: Context) -> None: """Create a plan in execute phase with queued state.""" step_create_plan_strategize_complete(context) - context.plan = context.lifecycle_service.execute_plan(context.plan.identity.plan_id) + if context.plan.phase == PlanPhase.STRATEGIZE: + context.plan = context.lifecycle_service.execute_plan( + context.plan.identity.plan_id + ) @given("I have a plan in execute phase with processing state") def step_create_plan_execute_processing(context: Context) -> None: """Create a plan in execute phase with processing state.""" step_create_plan_execute_queued(context) - context.plan = context.lifecycle_service.start_execute( - context.plan.identity.plan_id - ) + if context.plan.phase == PlanPhase.EXECUTE and ( + context.plan.processing_state == ProcessingState.QUEUED + ): + context.plan = context.lifecycle_service.start_execute( + context.plan.identity.plan_id + ) @given("I have a plan in execute phase with complete state") def step_create_plan_execute_complete(context: Context) -> None: """Create a plan in execute phase with complete state.""" step_create_plan_execute_processing(context) - context.plan = context.lifecycle_service.complete_execute( - context.plan.identity.plan_id - ) + if context.plan.phase == PlanPhase.EXECUTE and ( + context.plan.processing_state == ProcessingState.PROCESSING + ): + context.plan = context.lifecycle_service.complete_execute( + context.plan.identity.plan_id + ) @given("I have a plan in apply phase with queued state") def step_create_plan_apply_queued(context: Context) -> None: """Create a plan in apply phase with queued state.""" step_create_plan_execute_complete(context) - context.plan = context.lifecycle_service.apply_plan(context.plan.identity.plan_id) + if context.plan.phase == PlanPhase.EXECUTE: + context.plan = context.lifecycle_service.apply_plan( + context.plan.identity.plan_id + ) @given("I have a plan in apply phase with processing state") def step_create_plan_apply_processing(context: Context) -> None: """Create a plan in apply phase with processing state.""" step_create_plan_execute_complete(context) - context.plan = context.lifecycle_service.apply_plan(context.plan.identity.plan_id) - context.plan = context.lifecycle_service.start_apply(context.plan.identity.plan_id) + if context.plan.phase == PlanPhase.EXECUTE: + context.plan = context.lifecycle_service.apply_plan( + context.plan.identity.plan_id + ) + if context.plan.phase == PlanPhase.APPLY and ( + context.plan.processing_state == ProcessingState.QUEUED + ): + context.plan = context.lifecycle_service.start_apply( + context.plan.identity.plan_id + ) @given("I have a plan in applied phase") diff --git a/features/steps/tdd_use_action_automation_profile_steps.py b/features/steps/tdd_use_action_automation_profile_steps.py index b98128fb9..ff1ed9f19 100644 --- a/features/steps/tdd_use_action_automation_profile_steps.py +++ b/features/steps/tdd_use_action_automation_profile_steps.py @@ -23,11 +23,15 @@ from __future__ import annotations import tempfile from pathlib import Path +from typing import Any from behave import given, then, when from behave.runner import Context -from cleveragents.application.services.config_service import ConfigService +from cleveragents.application.services.config_service import ( + ConfigService, + ResolvedValue, +) from cleveragents.application.services.plan_lifecycle_service import ( PlanLifecycleService, ) @@ -62,11 +66,57 @@ def _use_action_on_project(context: Context, project: str) -> None: # --------------------------------------------------------------------------- +class _BrokenConfigService(ConfigService): + """A ConfigService subclass that always raises on resolve(). + + Used by the fallback-on-error scenario to verify that the exception + handler in ``_resolve_plan_profile_ref`` catches expected errors + and falls back to the settings default gracefully. + """ + + def resolve( + self, + key: str, + *, + cli_value: Any | None = None, + project_name: str | None = None, + verbose: bool = False, + ) -> ResolvedValue: + raise ValueError(f"Simulated config resolution failure for key '{key}'") + + +@given("a plan lifecycle service with a broken config service") +def step_create_lifecycle_service_broken_config(context: Context) -> None: + """Create a PlanLifecycleService with a config service that always errors.""" + tmpdir: str = tempfile.mkdtemp() + tmp_path: Path = Path(tmpdir) + context.ap_config_tmpdir = tmpdir + broken_config = _BrokenConfigService( + config_dir=tmp_path, + config_path=tmp_path / "config.toml", + ) + settings: Settings = Settings() + context.ap_lifecycle_service = PlanLifecycleService( + settings=settings, + config_service=broken_config, + ) + + @given("a plan lifecycle service for automation profile testing") def step_create_lifecycle_service(context: Context) -> None: """Create a PlanLifecycleService instance for automation profile tests.""" + tmpdir: str = tempfile.mkdtemp() + tmp_path: Path = Path(tmpdir) + context.ap_config_tmpdir = tmpdir + context.ap_config_service = ConfigService( + config_dir=tmp_path, + config_path=tmp_path / "config.toml", + ) settings: Settings = Settings() - context.ap_lifecycle_service = PlanLifecycleService(settings=settings) + context.ap_lifecycle_service = PlanLifecycleService( + settings=settings, + config_service=context.ap_config_service, + ) @given('an available action "{name}" with automation_profile "{profile}"') @@ -111,15 +161,9 @@ def step_set_project_scoped_profile( value at level 3. ``use_action()`` should consult this when the action itself has no automation_profile. """ - tmpdir: str = tempfile.mkdtemp() - context.ap_config_tmpdir = tmpdir - tmp_path: Path = Path(tmpdir) - config_svc: ConfigService = ConfigService( - config_dir=tmp_path, - config_path=tmp_path / "config.toml", + context.ap_config_service.set_project_value( + project, "core.automation-profile", profile ) - config_svc.set_project_value(project, "core.automation-profile", profile) - context.ap_config_service = config_svc # --------------------------------------------------------------------------- @@ -133,6 +177,20 @@ def step_use_profiled_action(context: Context, project: str) -> None: _use_action_on_project(context, project) +@when('I use the action with plan-level profile "{profile}" on project "{project}"') +def step_use_action_with_plan_profile( + context: Context, profile: str, project: str +) -> None: + """Use the action with an explicit plan-level profile override.""" + links: list[ProjectLink] = [ProjectLink(project_name=project)] + plan: Plan = context.ap_lifecycle_service.use_action( + action_name=str(context.ap_action.namespaced_name), + project_links=links, + automation_profile=profile, + ) + context.ap_plan = plan + + @when('I use the unprofiled action on project "{project}"') def step_use_unprofiled_action(context: Context, project: str) -> None: """Use the unprofiled action on a project via use_action().""" diff --git a/features/tdd_use_action_automation_profile.feature b/features/tdd_use_action_automation_profile.feature index d98039026..d85acd343 100644 --- a/features/tdd_use_action_automation_profile.feature +++ b/features/tdd_use_action_automation_profile.feature @@ -1,5 +1,5 @@ -@tdd_expected_fail @tdd_issue @tdd_issue_1076 -Feature: TDD Issue #1076 — use_action() does not propagate automation_profile to Plan +@tdd_bug @tdd_bug_1076 +Feature: TDD Bug #1076 — use_action() does not propagate automation_profile to Plan As a developer I want to verify that use_action() resolves the automation profile from the precedence chain and sets it on the created Plan @@ -50,3 +50,19 @@ Feature: TDD Issue #1076 — use_action() does not propagate automation_profile Then the created plan automation_profile should not be None And the created plan automation_profile name should be "supervised" And the created plan automation_profile provenance should be "global" + + Scenario: Plan falls back to settings default when config service raises an error + Given a plan lifecycle service with a broken config service + And an available action "local/fallback-action" without automation_profile + When I use the unprofiled action on project "test-project" + Then the created plan automation_profile should not be None + And the created plan automation_profile name should be "manual" + And the created plan automation_profile provenance should be "global" + + Scenario: Plan-level profile overrides action-level profile + Given a plan lifecycle service for automation profile testing + And an available action "local/override-action" with automation_profile "full-auto" + When I use the action with plan-level profile "ci" on project "test-project" + Then the created plan automation_profile should not be None + And the created plan automation_profile name should be "ci" + And the created plan automation_profile provenance should be "plan" diff --git a/robot/helper_int_wf05_db_migration.py b/robot/helper_int_wf05_db_migration.py index 2453aea22..2046ad6e1 100644 --- a/robot/helper_int_wf05_db_migration.py +++ b/robot/helper_int_wf05_db_migration.py @@ -193,10 +193,33 @@ def _review_profile_behavior() -> None: pid = plan.identity.plan_id service.start_strategize(pid) + p = service.get_plan(pid) + assert p.processing_state == ProcessingState.PROCESSING + service.complete_strategize(pid) - service.execute_plan(pid) - service.start_execute(pid) - plan = service.complete_execute(pid) + p = service.get_plan(pid) + assert (p.phase, p.processing_state) in { + (PlanPhase.STRATEGIZE, ProcessingState.COMPLETE), + (PlanPhase.EXECUTE, ProcessingState.QUEUED), + }, ( + "After complete_strategize expected strategize/complete or execute/queued, " + f"got {p.phase.value}/{p.processing_state.value}" + ) + if p.phase == PlanPhase.STRATEGIZE: + service.execute_plan(pid) + p = service.get_plan(pid) + + if p.phase == PlanPhase.EXECUTE and p.processing_state == ProcessingState.QUEUED: + service.start_execute(pid) + p = service.get_plan(pid) + + if ( + p.phase == PlanPhase.EXECUTE + and p.processing_state == ProcessingState.PROCESSING + ): + plan = service.complete_execute(pid) + else: + plan = p # Review: auto_progress must be False (apply gated) assert plan.phase == PlanPhase.EXECUTE @@ -207,8 +230,13 @@ def _review_profile_behavior() -> None: # Explicit apply works service.apply_plan(pid) - service.start_apply(pid) - plan = service.complete_apply(pid) + p = service.get_plan(pid) + if p.phase == PlanPhase.APPLY and p.processing_state == ProcessingState.QUEUED: + service.start_apply(pid) + plan = service.complete_apply(pid) + else: + plan = p + assert plan.processing_state == ProcessingState.APPLIED assert plan.is_terminal @@ -567,12 +595,34 @@ def _plan_lifecycle_review_profile() -> None: # Strategize service.start_strategize(pid) + p = service.get_plan(pid) + assert p.processing_state == ProcessingState.PROCESSING + service.complete_strategize(pid) + p = service.get_plan(pid) + assert (p.phase, p.processing_state) in { + (PlanPhase.STRATEGIZE, ProcessingState.COMPLETE), + (PlanPhase.EXECUTE, ProcessingState.QUEUED), + }, ( + "After complete_strategize expected strategize/complete or execute/queued, " + f"got {p.phase.value}/{p.processing_state.value}" + ) + if p.phase == PlanPhase.STRATEGIZE: + service.execute_plan(pid) + p = service.get_plan(pid) # Execute - service.execute_plan(pid) - service.start_execute(pid) - plan = service.complete_execute(pid) + if p.phase == PlanPhase.EXECUTE and p.processing_state == ProcessingState.QUEUED: + service.start_execute(pid) + p = service.get_plan(pid) + + if ( + p.phase == PlanPhase.EXECUTE + and p.processing_state == ProcessingState.PROCESSING + ): + plan = service.complete_execute(pid) + else: + plan = p # Review gate blocks auto-progress assert plan.phase == PlanPhase.EXECUTE @@ -581,8 +631,13 @@ def _plan_lifecycle_review_profile() -> None: # Explicit apply service.apply_plan(pid) - service.start_apply(pid) - plan = service.complete_apply(pid) + p = service.get_plan(pid) + if p.phase == PlanPhase.APPLY and p.processing_state == ProcessingState.QUEUED: + service.start_apply(pid) + plan = service.complete_apply(pid) + else: + plan = p + assert plan.phase == PlanPhase.APPLY assert plan.processing_state == ProcessingState.APPLIED assert plan.is_terminal diff --git a/robot/helper_wf07_cicd.py b/robot/helper_wf07_cicd.py index c1e61e58a..5d0d19669 100644 --- a/robot/helper_wf07_cicd.py +++ b/robot/helper_wf07_cicd.py @@ -372,8 +372,8 @@ def ci_plan_lifecycle() -> None: profile_svc = AutomationProfileService(repo=None) ci_profile = profile_svc.resolve_profile(plan_profile="ci") assert ci_profile.name == "ci" - # NOTE: TODO(#1060) - use_action() does not propagate action profile to plan. - # auto_progress then resolves to manual; keep phase guards until #1060 is fixed. + # use_action now resolves automation profile precedence and can auto-progress + # beyond phase boundaries for non-manual profiles (e.g., ci). # --- Phase-by-phase plan completion (spec Step 3) --- service.start_strategize(plan_id) p = service.get_plan(plan_id) @@ -398,15 +398,18 @@ def ci_plan_lifecycle() -> None: assert (p.phase, p.state) in { (PlanPhase.EXECUTE, ProcessingState.COMPLETE), (PlanPhase.APPLY, ProcessingState.QUEUED), + (PlanPhase.APPLY, ProcessingState.APPLIED), }, ( - "After complete_execute expected execute/complete or apply/queued, " + "After complete_execute expected execute/complete, apply/queued, " + "or apply/applied, " f"got {p.phase.value}/{p.state.value}" ) if p.phase == PlanPhase.EXECUTE: service.apply_plan(plan_id) # Apply phase - service.start_apply(plan_id) - service.complete_apply(plan_id) + if p.phase == PlanPhase.APPLY and p.state == ProcessingState.QUEUED: + service.start_apply(plan_id) + service.complete_apply(plan_id) # Verify terminal state — the polling loop exits on 'applied' final = service.get_plan(plan_id) assert final.state == ProcessingState.APPLIED, ( diff --git a/src/cleveragents/application/services/plan_lifecycle_service.py b/src/cleveragents/application/services/plan_lifecycle_service.py index ea7aff67a..84572bc9b 100644 --- a/src/cleveragents/application/services/plan_lifecycle_service.py +++ b/src/cleveragents/application/services/plan_lifecycle_service.py @@ -58,6 +58,7 @@ from typing import TYPE_CHECKING, Any import structlog from ulid import ULID +from cleveragents.application.services.config_service import ConfigLevel, ConfigService from cleveragents.application.services.plan_preflight_guardrail import ( PlanPreflightGuardrail, ) @@ -76,6 +77,8 @@ from cleveragents.domain.models.core.automation_profile import ( AutomationProfile, ) from cleveragents.domain.models.core.plan import ( + AutomationProfileProvenance, + AutomationProfileRef, InvariantSource, NamespacedName, Plan, @@ -166,6 +169,7 @@ class PlanLifecycleService: event_bus: EventBus | None = None, job_store: InMemoryJobStore | None = None, error_pattern_service: ErrorPatternService | None = None, + config_service: ConfigService | None = None, ): """Initialize the plan lifecycle service. @@ -201,6 +205,7 @@ class PlanLifecycleService: self.event_bus = event_bus self._job_store = job_store self.error_pattern_service = error_pattern_service + self._config_service = config_service self._logger = logger.bind(service="plan_lifecycle") self.preflight_guardrail = PlanPreflightGuardrail() @@ -726,6 +731,7 @@ class PlanLifecycleService: arguments: dict[str, Any] | None = None, created_by: str | None = None, invariants: list[PlanInvariant] | None = None, + automation_profile: str | None = None, ) -> Plan: """Use an action on projects to create a plan in Strategize phase. @@ -739,6 +745,7 @@ class PlanLifecycleService: arguments: Argument values for the action created_by: User/session creating the plan invariants: Additional plan-level invariants + automation_profile: Optional plan-level override profile name Returns: The created Plan in Strategize phase @@ -782,6 +789,11 @@ class PlanLifecycleService: plan_id = self._generate_ulid() action_full_name = str(action.namespaced_name) plan_name = f"{action.namespaced_name.name}-{plan_id[:8]}" + resolved_profile = self._resolve_plan_profile_ref( + plan_profile=automation_profile, + action_profile=action.automation_profile, + project_links=links, + ) plan = Plan( identity=PlanIdentity(plan_id=plan_id), @@ -815,6 +827,7 @@ class PlanLifecycleService: tags=action.tags.copy(), reusable=action.reusable, read_only=action.read_only, + automation_profile=resolved_profile, ) # Persist or store in-memory @@ -855,6 +868,100 @@ class PlanLifecycleService: return plan + def _resolve_plan_profile_ref( + self, + *, + plan_profile: str | None, + action_profile: str | None, + project_links: list[ProjectLink], + ) -> AutomationProfileRef: + """Resolve and record automation profile precedence for a new plan. + + Precedence chain: plan > action > project > global. + """ + normalized_plan_profile = ( + plan_profile.strip() if isinstance(plan_profile, str) else None + ) + normalized_action_profile = ( + action_profile.strip() if isinstance(action_profile, str) else None + ) + project_name = project_links[0].project_name if project_links else None + + project_profile: str | None = None + raw_settings_profile = getattr(self.settings, "default_automation_profile", "") + settings_fallback = ( + raw_settings_profile.strip() + if isinstance(raw_settings_profile, str) + else "" + ) + if not settings_fallback or settings_fallback not in BUILTIN_PROFILES: + settings_fallback = "manual" + global_profile: str = settings_fallback + if self._config_service is not None: + try: + global_resolved = self._config_service.resolve( + "core.automation-profile" + ) + if isinstance(global_resolved.value, str): + resolved_global_value = global_resolved.value.strip() + if resolved_global_value: + global_profile = resolved_global_value + + if project_name: + project_resolved = self._config_service.resolve( + "core.automation-profile", + project_name=project_name, + ) + if project_resolved.source is ConfigLevel.PROJECT and isinstance( + project_resolved.value, str + ): + candidate = project_resolved.value.strip() + if candidate: + project_profile = candidate + except (KeyError, ValueError, OSError) as exc: + self._logger.warning( + "Config resolution failed for automation profile, " + "falling back to settings default", + error=str(exc), + error_type=type(exc).__name__, + settings_fallback=settings_fallback, + ) + global_profile = settings_fallback + + chosen_profile = global_profile + provenance = AutomationProfileProvenance.GLOBAL + if project_profile: + chosen_profile = project_profile + provenance = AutomationProfileProvenance.PROJECT + if normalized_action_profile: + chosen_profile = normalized_action_profile + provenance = AutomationProfileProvenance.ACTION + if normalized_plan_profile: + chosen_profile = normalized_plan_profile + provenance = AutomationProfileProvenance.PLAN + + if chosen_profile not in BUILTIN_PROFILES: + raise ValidationError( + f"Unknown automation profile '{chosen_profile}'. " + f"Available built-ins: {', '.join(sorted(BUILTIN_PROFILES))}" + ) + + self._logger.info( + "Resolved automation profile for plan", + resolved_profile=chosen_profile, + provenance=provenance.value, + plan_profile=normalized_plan_profile, + action_profile=normalized_action_profile, + project_profile=project_profile, + global_profile=global_profile, + project_name=project_name, + ) + + return AutomationProfileRef( + profile_name=chosen_profile, + provenance=provenance, + ) + def get_plan(self, plan_id: str) -> Plan: """Get a plan by ID. diff --git a/src/cleveragents/cli/commands/plan.py b/src/cleveragents/cli/commands/plan.py index d225cb5bb..610ebc1bc 100644 --- a/src/cleveragents/cli/commands/plan.py +++ b/src/cleveragents/cli/commands/plan.py @@ -1662,6 +1662,18 @@ def use_action( # Get action by name action = service.get_action_by_name(action_name) + if automation_profile: + from cleveragents.domain.models.core.automation_profile import ( + BUILTIN_PROFILES, + ) + + if automation_profile not in BUILTIN_PROFILES: + console.print( + f"[red]Invalid automation profile:[/red] {automation_profile}. " + f"Available: {', '.join(sorted(BUILTIN_PROFILES))}" + ) + raise typer.Abort() + # Build project links from project names project_links = [ProjectLink(project_name=p) for p in all_projects] @@ -1679,6 +1691,7 @@ def use_action( project_links=project_links, arguments=arguments if arguments else None, invariants=plan_invariants, + automation_profile=automation_profile, ) # Notify A2A facade for protocol bookkeeping @@ -1694,18 +1707,10 @@ def use_action( # re-persist the plan when necessary. has_overrides = False - # Apply automation profile override if provided + # Preserve explicit CLI override locally as well so CLI output, + # persistence refresh, and mocked lifecycle-service tests all + # observe the plan-level profile consistently. if automation_profile: - from cleveragents.domain.models.core.automation_profile import ( - BUILTIN_PROFILES, - ) - - if automation_profile not in BUILTIN_PROFILES: - console.print( - f"[red]Invalid automation profile:[/red] {automation_profile}. " - f"Available: {', '.join(sorted(BUILTIN_PROFILES))}" - ) - raise typer.Abort() plan.automation_profile = AutomationProfileRef( profile_name=automation_profile, provenance=AutomationProfileProvenance.PLAN,