forked from HAL9000/cleveragents-core
774dfedc6b
## Summary This PR adds TDD bug-capture tests for bug #967 — `plan execute` only transitions state without running strategize or execute phase processing. ### Motivation Bug #967 describes that the `plan execute` CLI command originally only called `service.execute_plan(plan_id)`, which is a state transition only (Strategize/COMPLETE → Execute/QUEUED). When a plan was in Strategize/QUEUED state (immediately after `plan use`), the command failed because `execute_plan()` requires Strategize/COMPLETE. The CLI should detect the plan's current phase and run `PlanExecutor.run_strategize()` before transitioning. Per the project's TDD Bug Fix Workflow (`CONTRIBUTING.md`), the first step in fixing any bug is to write a test that captures the buggy behavior. Since the fix for #967 is already present in the codebase (the CLI handler already orchestrates properly), the `@tdd_expected_fail` tags have been removed and these tests serve as **permanent regression guards** ensuring the fix is never reverted. ### Design Approach **All `@tdd_expected_fail` scenarios were rewritten to exercise the CLI orchestration layer** — the actual code path affected by bug #967. This satisfies AC4: "The test is specific enough that it will pass normally (without the tag) only when the bug is genuinely fixed." - **Scenarios 1, 2, 4** (previously `@tdd_expected_fail`): Use Typer's `CliRunner` with mocked services to invoke the `plan execute` CLI command handler directly. This tests the orchestration logic in `plan.py` — the exact code that was buggy. - **Scenario 3** (positive control): Uses real `PlanLifecycleService` (in-memory) and `PlanExecutor` (stub actors) to demonstrate that proper service-level orchestration works. - **Robot tests**: Replicate the CLI orchestration logic using real services to verify at the integration level. ### Changes #### Behave Unit Tests - `features/tdd_plan_execute_phase_processing.feature` — 4 scenarios - `features/steps/tdd_plan_execute_phase_processing_steps.py` — Step definitions using CliRunner and mocked services **Scenarios:** 1. **CLI execute command handles plan in Strategize/QUEUED state** — Invokes `plan execute` via CliRunner on a QUEUED plan. Verifies the CLI succeeds and the plan reaches Execute phase. 2. **CLI execute command orchestrates full lifecycle for QUEUED plan** — Verifies `run_strategize()` and `run_execute()` are both called by the CLI handler. 3. **Positive control — proper orchestration transitions QUEUED plan to Execute** — Demonstrates that `run_strategize()` → `execute_plan()` works correctly at the service level. Always passes. 4. **CLI auto-discovery finds plans in Strategize/QUEUED state** — Invokes `plan execute` with no plan_id. Verifies the auto-discovery filter includes QUEUED plans. #### Robot Integration Tests - `robot/tdd_plan_execute_phase_processing.robot` — 4 test cases matching the Behave scenarios - `robot/helper_tdd_plan_execute_phase_processing.py` — Helper script replicating CLI orchestration logic #### CHANGELOG - `CHANGELOG.md` — Added entry under "## Unreleased" describing the new tests. ### Quality Gates - `nox -e lint`: ✅ passed - `nox -e typecheck`: ✅ passed (0 errors) - `nox -e unit_tests`: ✅ passed (391 features, 11177 scenarios, 0 failures) - `nox -e integration_tests`: ✅ passed (1572 tests, 0 failures) - `nox -e e2e_tests`: ✅ passed (16 tests, 0 failures) - `nox -e coverage_report`: ✅ 97% coverage ### Review Cycle 2 Fixes - **Critical #1**: Rewrote `@tdd_expected_fail` scenarios to exercise the CLI orchestration layer via CliRunner instead of testing service/executor APIs that are correct by design. Removed `@tdd_expected_fail` tags since the bug fix is already in the codebase. - **Major #2**: Added CHANGELOG entry. - **Major #3**: Rebased onto current `master`. - **Minor #4**: Added `ProcessingState.QUEUED` assertion to positive control scenario. - **Minor #5**: Narrowed exception handler from bare `except Exception` to `except (PlanError, PlanNotReadyError)`. - **Minor #7**: Changed Robot suite to use `Setup Test Environment With Database Isolation`. - **Minor #8**: Added `on_timeout=kill` to all Robot `Run Process` calls. - **Minor #12**: Added docstring to `_fail()` helper. - **Minor #13**: Removed redundant `Settings()` instantiation. ### Known Limitations - The `@tdd_expected_fail` tags were removed because the bug fix for #967 is already in the codebase. If this PR is merged before the #967 fix PR, the tags would need to be re-added. However, the CHANGELOG and existing CLI code confirm the fix is already on `master`. Closes #977 Reviewed-on: cleveragents/cleveragents-core#1050 Co-authored-by: Rui Hu <rui.hu@cleverthis.com> Co-committed-by: Rui Hu <rui.hu@cleverthis.com>
218 lines
8.4 KiB
Python
218 lines
8.4 KiB
Python
"""Helper script for tdd_plan_execute_phase_processing.robot smoke tests.
|
|
|
|
Each subcommand exercises the **CLI orchestration layer** by replicating the
|
|
logic of the ``execute_plan`` CLI handler in ``plan.py``. The helper uses
|
|
a real ``PlanLifecycleService`` (in-memory mode) and ``PlanExecutor`` (with
|
|
stub actors) to verify that the orchestration correctly handles plans in
|
|
Strategize/QUEUED state.
|
|
|
|
Bug #967: The ``plan execute`` CLI command originally only called
|
|
``service.execute_plan(plan_id)``, which is a state transition only
|
|
(Strategize/COMPLETE → Execute/QUEUED). It did not construct a
|
|
``PlanExecutor`` or call ``run_strategize()`` / ``run_execute()``. The fix
|
|
added phase-aware orchestration; these tests verify it works.
|
|
|
|
This test was written to capture bug #967 per ticket #977. The bug fix is
|
|
already in the codebase, so these tests serve as permanent regression guards.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import sys
|
|
from collections.abc import Callable
|
|
from pathlib import Path
|
|
from typing import NoReturn
|
|
|
|
_ROOT: Path = Path(__file__).resolve().parents[1]
|
|
_SRC: str = str(_ROOT / "src")
|
|
if _SRC not in sys.path:
|
|
sys.path.insert(0, _SRC)
|
|
|
|
from cleveragents.application.services.plan_executor import PlanExecutor # noqa: E402
|
|
from cleveragents.application.services.plan_lifecycle_service import ( # noqa: E402
|
|
PlanLifecycleService,
|
|
)
|
|
from cleveragents.config.settings import Settings # noqa: E402
|
|
from cleveragents.domain.models.core.plan import ( # noqa: E402
|
|
Plan,
|
|
PlanPhase,
|
|
ProcessingState,
|
|
)
|
|
|
|
|
|
def _fail(msg: str) -> NoReturn:
|
|
"""Print an error message to stderr and exit with code 1."""
|
|
print(msg, file=sys.stderr)
|
|
sys.exit(1)
|
|
|
|
|
|
def _create_service_and_queued_plan() -> tuple[PlanLifecycleService, str]:
|
|
"""Create a real in-memory PlanLifecycleService and a plan in Strategize/QUEUED."""
|
|
service: PlanLifecycleService = PlanLifecycleService(settings=Settings())
|
|
action = service.create_action(
|
|
name="local/tdd-967-robot-action",
|
|
description="TDD action for bug #967 (Robot)",
|
|
definition_of_done="Complete the test plan",
|
|
strategy_actor="local/stub-strategize",
|
|
execution_actor="local/stub-execute",
|
|
)
|
|
plan: Plan = service.use_action(action_name=str(action.namespaced_name))
|
|
return service, plan.identity.plan_id
|
|
|
|
|
|
def _cli_execute_from_queued() -> None:
|
|
"""Test the CLI orchestration path for a plan in Strategize/QUEUED.
|
|
|
|
Replicates the ``execute_plan`` CLI handler logic: detect the plan is
|
|
in Strategize/QUEUED, run strategize via ``PlanExecutor``, then
|
|
transition to Execute phase.
|
|
"""
|
|
service, plan_id = _create_service_and_queued_plan()
|
|
executor: PlanExecutor = PlanExecutor(lifecycle_service=service)
|
|
|
|
# Replicate CLI orchestration: detect phase, run strategize if QUEUED
|
|
plan: Plan = service.get_plan(plan_id)
|
|
if plan.phase == PlanPhase.STRATEGIZE and plan.state == ProcessingState.QUEUED:
|
|
executor.run_strategize(plan_id)
|
|
plan = service.get_plan(plan_id)
|
|
|
|
# Transition to Execute if still in Strategize/COMPLETE
|
|
if plan.phase == PlanPhase.STRATEGIZE and plan.state == ProcessingState.COMPLETE:
|
|
plan = service.execute_plan(plan_id)
|
|
elif plan.phase != PlanPhase.EXECUTE:
|
|
_fail(
|
|
f"Bug #967: CLI orchestration failed. Plan in unexpected state "
|
|
f"after strategize: {plan.phase.value}/{plan.state.value}"
|
|
)
|
|
|
|
if plan.phase != PlanPhase.EXECUTE:
|
|
_fail(f"Bug #967: Expected Execute phase, got {plan.phase.value}")
|
|
print("tdd-cli-execute-from-queued-ok")
|
|
|
|
|
|
def _cli_full_orchestration() -> None:
|
|
"""Test the CLI orchestration runs both strategize and execute phases.
|
|
|
|
Replicates the full ``execute_plan`` CLI handler: strategize → transition
|
|
→ run_execute.
|
|
"""
|
|
service, plan_id = _create_service_and_queued_plan()
|
|
executor: PlanExecutor = PlanExecutor(lifecycle_service=service)
|
|
|
|
# Step 1: CLI detects Strategize/QUEUED and runs strategize
|
|
plan: Plan = service.get_plan(plan_id)
|
|
if plan.phase == PlanPhase.STRATEGIZE and plan.state in (
|
|
ProcessingState.QUEUED,
|
|
ProcessingState.PROCESSING,
|
|
):
|
|
executor.run_strategize(plan_id)
|
|
plan = service.get_plan(plan_id)
|
|
|
|
# Step 2: Transition to Execute if needed
|
|
if plan.phase == PlanPhase.STRATEGIZE and plan.state == ProcessingState.COMPLETE:
|
|
plan = service.execute_plan(plan_id)
|
|
elif plan.phase != PlanPhase.EXECUTE:
|
|
_fail(
|
|
f"Bug #967: Unexpected state after strategize: "
|
|
f"{plan.phase.value}/{plan.state.value}"
|
|
)
|
|
|
|
# Step 3: Run execute phase
|
|
plan = service.get_plan(plan_id)
|
|
if plan.phase == PlanPhase.EXECUTE and plan.state == ProcessingState.QUEUED:
|
|
executor.run_execute(plan_id)
|
|
plan = service.get_plan(plan_id)
|
|
|
|
if plan.phase != PlanPhase.EXECUTE:
|
|
_fail(f"Bug #967: Expected Execute phase, got {plan.phase.value}")
|
|
if plan.state not in (ProcessingState.COMPLETE, ProcessingState.QUEUED):
|
|
_fail(f"Bug #967: Expected COMPLETE or QUEUED state, got {plan.state.value}")
|
|
print("tdd-cli-full-orchestration-ok")
|
|
|
|
|
|
def _proper_orchestration() -> None:
|
|
"""Positive control: proper service-level orchestration works on a QUEUED plan."""
|
|
service, plan_id = _create_service_and_queued_plan()
|
|
executor: PlanExecutor = PlanExecutor(lifecycle_service=service)
|
|
|
|
# Run strategize first (QUEUED → PROCESSING → COMPLETE)
|
|
executor.run_strategize(plan_id)
|
|
|
|
# Re-fetch — auto_progress may have moved it to Execute
|
|
plan: Plan = service.get_plan(plan_id)
|
|
if plan.phase == PlanPhase.STRATEGIZE and plan.state == ProcessingState.COMPLETE:
|
|
plan = service.execute_plan(plan_id)
|
|
elif plan.phase != PlanPhase.EXECUTE:
|
|
_fail(
|
|
f"Bug #967: Unexpected state after orchestration: "
|
|
f"{plan.phase.value}/{plan.state.value}"
|
|
)
|
|
|
|
plan = service.get_plan(plan_id)
|
|
if plan.phase != PlanPhase.EXECUTE:
|
|
_fail(f"Bug #967: Expected Execute phase, got {plan.phase.value}")
|
|
if plan.state != ProcessingState.QUEUED:
|
|
_fail(f"Bug #967: Expected QUEUED state, got {plan.state.value}")
|
|
print("tdd-proper-orchestration-ok")
|
|
|
|
|
|
def _cli_auto_discovery() -> None:
|
|
"""Test the CLI auto-discovery filter includes Strategize/QUEUED plans.
|
|
|
|
Replicates the CLI auto-discovery logic from the ``execute_plan`` handler:
|
|
list plans and filter for eligible states including QUEUED.
|
|
"""
|
|
service: PlanLifecycleService = PlanLifecycleService(settings=Settings())
|
|
|
|
# Create an action
|
|
action = service.create_action(
|
|
name="local/tdd-967-discovery",
|
|
description="TDD action for auto-discovery (Robot)",
|
|
definition_of_done="Test auto-discovery",
|
|
strategy_actor="local/stub-strategize",
|
|
execution_actor="local/stub-execute",
|
|
)
|
|
action_name: str = str(action.namespaced_name)
|
|
|
|
# Plan 1: Strategize/QUEUED (fresh from use_action)
|
|
plan_queued: Plan = service.use_action(action_name=action_name)
|
|
queued_id: str = plan_queued.identity.plan_id
|
|
|
|
# Apply the PRODUCTION auto-discovery filter (the fixed version):
|
|
# includes both QUEUED and COMPLETE Strategize plans
|
|
plans_strat: list[Plan] = service.list_plans(phase=PlanPhase.STRATEGIZE)
|
|
plans_exec: list[Plan] = service.list_plans(phase=PlanPhase.EXECUTE)
|
|
eligible: list[Plan] = [
|
|
p
|
|
for p in plans_strat
|
|
if p.state in (ProcessingState.QUEUED, ProcessingState.COMPLETE)
|
|
] + [p for p in plans_exec if p.state == ProcessingState.QUEUED]
|
|
|
|
queued_in_eligible: list[Plan] = [
|
|
p for p in eligible if p.identity.plan_id == queued_id
|
|
]
|
|
if len(queued_in_eligible) == 0:
|
|
_fail(
|
|
"Bug #967: Auto-discovery filter does not find Strategize/QUEUED "
|
|
f"plans. Eligible count: {len(eligible)}, QUEUED plan: {queued_id}"
|
|
)
|
|
print("tdd-cli-auto-discovery-ok")
|
|
|
|
|
|
_COMMANDS: dict[str, Callable[[], None]] = {
|
|
"cli-execute-from-queued": _cli_execute_from_queued,
|
|
"cli-full-orchestration": _cli_full_orchestration,
|
|
"proper-orchestration": _proper_orchestration,
|
|
"cli-auto-discovery": _cli_auto_discovery,
|
|
}
|
|
|
|
if __name__ == "__main__":
|
|
if len(sys.argv) < 2 or sys.argv[1] not in _COMMANDS:
|
|
print(
|
|
f"Usage: {sys.argv[0]} <{'|'.join(_COMMANDS)}>",
|
|
file=sys.stderr,
|
|
)
|
|
sys.exit(1)
|
|
cmd: Callable[[], None] = _COMMANDS[sys.argv[1]]
|
|
cmd()
|