From c0d631b6ccf8f8c0addef3fdbb07ce6bc25fd02c Mon Sep 17 00:00:00 2001 From: CleverThis Date: Mon, 13 Apr 2026 08:17:31 +0000 Subject: [PATCH 1/2] fix: custom automation profiles silently fall back to manual _resolve_profile_for_plan in PlanLifecycleService now raises a clear ValidationError when a plan's automation profile name is not a known built-in profile, instead of silently falling back to 'manual'. Users who configured custom automation profiles (e.g. 'semi-auto', 'acme/strict') will now receive an actionable error message listing available built-in profiles. The resolved profile name is also logged at debug level for observability. Added BDD regression tests (8 scenarios) covering: - Built-in profiles resolve correctly - Plans with no profile set resolve to 'manual' - Custom/unknown profile names raise ValidationError - Error message mentions the unknown profile name - Error message lists available built-in profiles - Debug log records the resolved profile name ISSUES CLOSED: #8232 --- CHANGELOG.md | 10 + ..._profile_for_plan_silent_fallback_steps.py | 244 ++++++++++++++++++ ...e_profile_for_plan_silent_fallback.feature | 67 +++++ .../services/plan_lifecycle_service.py | 25 +- 4 files changed, 345 insertions(+), 1 deletion(-) create mode 100644 features/steps/tdd_resolve_profile_for_plan_silent_fallback_steps.py create mode 100644 features/tdd_resolve_profile_for_plan_silent_fallback.feature diff --git a/CHANGELOG.md b/CHANGELOG.md index 35247aff4..ade2d531a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,16 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Fixed + +- **Automation Profile Silent Fallback** (#8232): `_resolve_profile_for_plan` in + `PlanLifecycleService` now raises a clear `ValidationError` when a plan's + automation profile name is not a known built-in profile, instead of silently + falling back to `"manual"`. Users who configured custom automation profiles + (e.g. `"semi-auto"`, `"acme/strict"`) will now receive an actionable error + message listing available built-in profiles. The resolved profile name is also + logged at debug level for observability. + ### Added - **TDD Issue-Capture Test Activation** (#7025): Replaced 234 bare `@skip` tags diff --git a/features/steps/tdd_resolve_profile_for_plan_silent_fallback_steps.py b/features/steps/tdd_resolve_profile_for_plan_silent_fallback_steps.py new file mode 100644 index 000000000..96293ef7a --- /dev/null +++ b/features/steps/tdd_resolve_profile_for_plan_silent_fallback_steps.py @@ -0,0 +1,244 @@ +"""Step definitions for TDD Bug #8232 — _resolve_profile_for_plan silent fallback. + +These steps exercise ``PlanLifecycleService._resolve_profile_for_plan()`` and +verify that: + +1. Built-in profiles resolve correctly. +2. A plan with no automation_profile set resolves to "manual". +3. Custom (unknown) profile names raise ``ValidationError`` instead of + silently falling back to "manual". +4. The resolved profile name is logged at debug level. + +Bug #8232: ``_resolve_profile_for_plan`` used ``BUILTIN_PROFILES.get(name, +BUILTIN_PROFILES["manual"])`` which silently returned the "manual" profile +when a custom profile name was not found. Users who configured custom +profiles (e.g. "semi-auto", "full-auto") found their plans executing in +manual mode without any warning. + +All step text uses the ``rp8232`` prefix to avoid conflicts with other +step definitions in the test suite. +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import MagicMock + +from behave import given, then, when +from behave.runner import Context +from ulid import ULID + +from cleveragents.application.services.plan_lifecycle_service import ( + PlanLifecycleService, +) +from cleveragents.config.settings import Settings +from cleveragents.core.exceptions import ValidationError +from cleveragents.domain.models.core.automation_profile import BUILTIN_PROFILES +from cleveragents.domain.models.core.plan import ( + AutomationProfileProvenance, + AutomationProfileRef, + NamespacedName, + Plan, + PlanIdentity, + PlanTimestamps, +) + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_plan_rp8232(profile_name: str | None = None) -> Plan: + """Create a minimal Plan for testing _resolve_profile_for_plan. + + Args: + profile_name: If provided, sets ``automation_profile`` to an + ``AutomationProfileRef`` with this name. If ``None``, the + plan's ``automation_profile`` is left as ``None``. + + Returns: + A minimal ``Plan`` instance. + """ + automation_profile: AutomationProfileRef | None = None + if profile_name is not None: + automation_profile = AutomationProfileRef( + profile_name=profile_name, + provenance=AutomationProfileProvenance.PLAN, + ) + + return Plan( + identity=PlanIdentity(plan_id=str(ULID())), + namespaced_name=NamespacedName( + server="local", + namespace="test", + name="test-plan", + ), + action_name="local/test-action", + description="Test plan for profile resolution", + definition_of_done="Tests pass", + strategy_actor="openai/gpt-4", + execution_actor="openai/gpt-4", + timestamps=PlanTimestamps(), + automation_profile=automation_profile, + ) + + +# --------------------------------------------------------------------------- +# Given steps +# --------------------------------------------------------------------------- + + +@given("rp8232 a fresh PlanLifecycleService for profile resolution testing") +def step_rp8232_create_fresh_service(context: Context) -> None: + """Create a fresh PlanLifecycleService for profile resolution tests.""" + Settings._instance = None + settings: Settings = Settings() + context.rp8232_service = PlanLifecycleService(settings=settings) + context.rp8232_raised_error: Exception | None = None + context.rp8232_resolved_profile_name: str | None = None + context.rp8232_debug_log_calls: list[dict[str, Any]] = [] + + +@given('rp8232 a plan with automation_profile "{profile_name}"') +def step_rp8232_create_plan_with_profile(context: Context, profile_name: str) -> None: + """Create a plan with the given automation_profile name.""" + context.rp8232_plan = _make_plan_rp8232(profile_name=profile_name) + + +@given("rp8232 a plan with no automation_profile set") +def step_rp8232_create_plan_without_profile(context: Context) -> None: + """Create a plan with automation_profile=None.""" + context.rp8232_plan = _make_plan_rp8232(profile_name=None) + + +# --------------------------------------------------------------------------- +# When steps +# --------------------------------------------------------------------------- + + +@when("rp8232 _resolve_profile_for_plan is called on the plan") +def step_rp8232_call_resolve_profile(context: Context) -> None: + """Call _resolve_profile_for_plan and capture result or exception.""" + context.rp8232_raised_error = None + context.rp8232_resolved_profile_name = None + context.rp8232_debug_log_calls = [] + + # Patch the structlog logger to capture debug calls + original_debug = context.rp8232_service._logger.debug + + def _capture_debug(event: str, **kwargs: Any) -> None: + context.rp8232_debug_log_calls.append({"event": event, **kwargs}) + original_debug(event, **kwargs) + + mock_logger = MagicMock() + mock_logger.debug.side_effect = _capture_debug + context.rp8232_service._logger = mock_logger + + try: + profile = context.rp8232_service._resolve_profile_for_plan(context.rp8232_plan) + context.rp8232_resolved_profile_name = profile.name + except ValidationError as exc: + context.rp8232_raised_error = exc + + +# --------------------------------------------------------------------------- +# Then steps +# --------------------------------------------------------------------------- + + +@then('rp8232 the resolved profile name should be "{expected_name}"') +def step_rp8232_assert_resolved_profile_name( + context: Context, expected_name: str +) -> None: + """Assert the resolved profile has the expected name.""" + assert context.rp8232_raised_error is None, ( + f"Expected no error but got ValidationError: {context.rp8232_raised_error}" + ) + assert context.rp8232_resolved_profile_name == expected_name, ( + f"Expected resolved profile '{expected_name}', " + f"got '{context.rp8232_resolved_profile_name}'. " + "_resolve_profile_for_plan did not resolve the correct profile." + ) + + +@then("rp8232 no ValidationError should have been raised") +def step_rp8232_assert_no_error(context: Context) -> None: + """Assert no ValidationError was raised.""" + assert context.rp8232_raised_error is None, ( + f"Expected no ValidationError but got: {context.rp8232_raised_error}" + ) + + +@then("rp8232 a ValidationError should be raised") +def step_rp8232_assert_validation_error_raised(context: Context) -> None: + """Assert a ValidationError was raised for the unknown profile. + + Bug #8232: The old code silently returned BUILTIN_PROFILES["manual"] + instead of raising an error when a custom profile name was not found. + """ + assert context.rp8232_raised_error is not None, ( + "Expected a ValidationError to be raised for an unknown custom profile, " + "but no error was raised. " + "Bug #8232: _resolve_profile_for_plan silently falls back to 'manual' " + "instead of raising a clear error for unknown profile names." + ) + assert isinstance(context.rp8232_raised_error, ValidationError), ( + f"Expected ValidationError, got {type(context.rp8232_raised_error).__name__}: " + f"{context.rp8232_raised_error}" + ) + + +@then('rp8232 the error message should mention "{profile_name}"') +def step_rp8232_assert_error_mentions_profile( + context: Context, profile_name: str +) -> None: + """Assert the error message mentions the unknown profile name.""" + assert context.rp8232_raised_error is not None, ( + "No error was raised — cannot check error message." + ) + error_msg = str(context.rp8232_raised_error) + assert profile_name in error_msg, ( + f"Expected error message to mention '{profile_name}', but got: {error_msg!r}" + ) + + +@then("rp8232 the error message should list available built-in profiles") +def step_rp8232_assert_error_lists_profiles(context: Context) -> None: + """Assert the error message lists the available built-in profiles.""" + assert context.rp8232_raised_error is not None, ( + "No error was raised — cannot check error message." + ) + error_msg = str(context.rp8232_raised_error) + # At least one known built-in profile name should appear in the error + known_profiles = list(BUILTIN_PROFILES.keys()) + found_any = any(p in error_msg for p in known_profiles) + assert found_any, ( + f"Expected error message to list available built-in profiles " + f"(e.g. {known_profiles[:3]}), but got: {error_msg!r}" + ) + + +@then('rp8232 the debug log should record the resolved profile name "{expected_name}"') +def step_rp8232_assert_debug_log_records_profile( + context: Context, expected_name: str +) -> None: + """Assert the debug log recorded the resolved profile name. + + The fix adds a debug log call after successful profile resolution so + that operators can trace which profile was used for each plan. + """ + assert context.rp8232_raised_error is None, ( + f"Expected no error but got: {context.rp8232_raised_error}" + ) + debug_calls = context.rp8232_debug_log_calls + matching = [ + call + for call in debug_calls + if call.get("event") == "resolved_automation_profile_for_plan" + and call.get("profile_name") == expected_name + ] + assert matching, ( + f"Expected a debug log call with event='resolved_automation_profile_for_plan' " + f"and profile_name='{expected_name}', but found: {debug_calls!r}. " + "The fix should log the resolved profile name at debug level." + ) diff --git a/features/tdd_resolve_profile_for_plan_silent_fallback.feature b/features/tdd_resolve_profile_for_plan_silent_fallback.feature new file mode 100644 index 000000000..0314fd8ea --- /dev/null +++ b/features/tdd_resolve_profile_for_plan_silent_fallback.feature @@ -0,0 +1,67 @@ +@tdd_issue @tdd_issue_8232 +Feature: TDD Bug #8232 — _resolve_profile_for_plan silently falls back to manual + As a developer + I want _resolve_profile_for_plan to raise a clear error for unknown profiles + So that custom automation profiles are never silently replaced with 'manual' + + Per the v3.5.0 acceptance criterion: + "Automation profile resolution precedence correct (plan > action > global)" + + The bug: when a plan's automation_profile.profile_name is a custom name + not present in BUILTIN_PROFILES, the old code silently returned + BUILTIN_PROFILES["manual"] instead of raising an error. + + The fix: raise ValidationError for unknown profile names and log the + resolved profile name at debug level. + + Background: + Given rp8232 a fresh PlanLifecycleService for profile resolution testing + + Scenario: Built-in profile resolves correctly without fallback + Given rp8232 a plan with automation_profile "full-auto" + When rp8232 _resolve_profile_for_plan is called on the plan + Then rp8232 the resolved profile name should be "full-auto" + And rp8232 no ValidationError should have been raised + + Scenario: Plan with no automation_profile resolves to manual + Given rp8232 a plan with no automation_profile set + When rp8232 _resolve_profile_for_plan is called on the plan + Then rp8232 the resolved profile name should be "manual" + And rp8232 no ValidationError should have been raised + + Scenario: Custom profile name raises ValidationError instead of silent fallback + Given rp8232 a plan with automation_profile "semi-auto" + When rp8232 _resolve_profile_for_plan is called on the plan + Then rp8232 a ValidationError should be raised + And rp8232 the error message should mention "semi-auto" + And rp8232 the error message should list available built-in profiles + + Scenario: Another custom profile name raises ValidationError + Given rp8232 a plan with automation_profile "acme/strict" + When rp8232 _resolve_profile_for_plan is called on the plan + Then rp8232 a ValidationError should be raised + And rp8232 the error message should mention "acme/strict" + + Scenario: All built-in profiles resolve without error + Given rp8232 a plan with automation_profile "manual" + When rp8232 _resolve_profile_for_plan is called on the plan + Then rp8232 the resolved profile name should be "manual" + And rp8232 no ValidationError should have been raised + + Scenario: Built-in profile "ci" resolves correctly + Given rp8232 a plan with automation_profile "ci" + When rp8232 _resolve_profile_for_plan is called on the plan + Then rp8232 the resolved profile name should be "ci" + And rp8232 no ValidationError should have been raised + + Scenario: Built-in profile "auto" resolves correctly + Given rp8232 a plan with automation_profile "auto" + When rp8232 _resolve_profile_for_plan is called on the plan + Then rp8232 the resolved profile name should be "auto" + And rp8232 no ValidationError should have been raised + + Scenario: Resolved profile name is logged at debug level + Given rp8232 a plan with automation_profile "trusted" + When rp8232 _resolve_profile_for_plan is called on the plan + Then rp8232 the resolved profile name should be "trusted" + And rp8232 the debug log should record the resolved profile name "trusted" diff --git a/src/cleveragents/application/services/plan_lifecycle_service.py b/src/cleveragents/application/services/plan_lifecycle_service.py index f63a4215d..846c110e5 100644 --- a/src/cleveragents/application/services/plan_lifecycle_service.py +++ b/src/cleveragents/application/services/plan_lifecycle_service.py @@ -2122,17 +2122,40 @@ class PlanLifecycleService: Uses the plan's ``automation_profile`` reference. Falls back to the ``"manual"`` built-in profile when no profile is set. + Custom profile names that are not found in ``BUILTIN_PROFILES`` + raise a ``ValidationError`` rather than silently falling back to + ``"manual"``. This ensures misconfigured custom profiles are + surfaced immediately instead of executing in an unintended mode. + Args: plan: The plan to resolve the profile for. Returns: The resolved ``AutomationProfile``. + + Raises: + ValidationError: If the plan's profile name is not a known + built-in profile. """ if plan.automation_profile is not None: profile_name = plan.automation_profile.profile_name else: profile_name = "manual" - return BUILTIN_PROFILES.get(profile_name, BUILTIN_PROFILES["manual"]) + + profile = BUILTIN_PROFILES.get(profile_name) + if profile is None: + raise ValidationError( + f"Automation profile '{profile_name}' is not a known built-in profile. " + f"Available profiles: {', '.join(sorted(BUILTIN_PROFILES))}. " + "Custom profiles must be registered before use." + ) + + self._logger.debug( + "resolved_automation_profile_for_plan", + plan_id=plan.identity.plan_id, + profile_name=profile_name, + ) + return profile def should_auto_progress(self, plan: Plan) -> bool: """Check whether the plan should automatically advance. -- 2.52.0 From 836b0e19c310ee1802e95560a9f78c422fd3e14d Mon Sep 17 00:00:00 2001 From: CleverThis Date: Mon, 13 Apr 2026 18:17:11 +0000 Subject: [PATCH 2/2] docs: confirm HAL 9000 contributor entry in CONTRIBUTORS.md HAL 9000 was already listed in the Contributors section. This commit adds a detail entry in the Details section to explicitly document the nature of HAL 9000's contributions (automated implementation, bug fixes, and feature development), satisfying the CONTRIBUTING.md requirement to update CONTRIBUTORS.md when making a change. ISSUES CLOSED: #8232 --- CONTRIBUTORS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 0b43e1538..62a76a4bc 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -14,4 +14,5 @@ Below are some of the specific details of various contributions. * Jeffrey Phillips Freeman has acted as Lead Developer, daily contributor, and Project Owner. * Brent E. Edwards has contributed quality assurance, test coverage, and CI pipeline improvements. +* HAL 9000 has contributed automated implementation, bug fixes, and feature development as part of the CleverAgents automation pool. * This project was made possible thanks to considerable donation of time, money, and resources by CleverThis, Inc. -- 2.52.0