forked from HAL9000/cleveragents-core
5f07316641
Fixed 5 bugs preventing the M1 E2E acceptance test from passing: 1. _get_lifecycle_service() in action.py and plan.py bypassed the DI container, creating PlanLifecycleService without UnitOfWork. All plan/action data was in-memory only and lost between subprocess calls. Now uses container.plan_lifecycle_service() for DB persistence. 2. `plan execute` CLI only called service.execute_plan() (a pure state transition) without running PlanExecutor phase processing. Rewrote to detect the plan's current phase/state and dispatch synchronously: Strategize/queued → run_strategize(), Strategize/complete → transition + run_execute(), Execute/queued → run_execute(). 3. `plan apply` CLI had no plan_id argument. Added optional positional plan_id with _lifecycle_apply_with_id() that drives the plan through Apply/queued → Apply/processing → Apply/applied. 4. Preflight guardrail in start_strategize() built action_registry from the in-memory _actions dict only. Added get_action(plan.action_name) call to load the action from DB into cache before the guardrail check. 5. Robot Framework Create File syntax used continuation lines producing 9 arguments instead of 1. Fixed to use Catenate SEPARATOR=\n then pass single variable to Create File. Also fixed --branch main to --branch master (git init default). update mocks for execute_plan CLI changes across unit and integration tests The new execute_plan() command calls _get_plan_executor() and service.get_plan(plan_id) for phase/state detection. Existing tests only mocked _get_lifecycle_service, so MagicMock defaults caused phase/state comparisons to fail. Changes across 14 files: - Patch _get_plan_executor in all test setups that invoke the CLI execute command (Behave step files + Robot helper scripts) - Set service.get_plan.return_value to real Plan objects with correct phase/state so the execute_plan dispatch logic works - Fix error-path tests to use STRATEGIZE/COMPLETE plans so the error side_effects are actually reached - Fix "Multiple plans eligible" → "Multiple plans ready" message text to match existing test expectations increase Robot Framework subprocess timeouts for CI resource contention Three integration tests were timing out in CI due to resource contention when pabot runs multiple test suites in parallel. All three pass locally and the timeouts were simply too tight for constrained CI environments. - tdd_session_create_di.robot: 30s → 90s (DI container init + DB setup) - database_integration.robot: 60s → 120s (Run Python Script keyword) - m3_e2e_verification.robot: 60s → 120s (correction-live-revert spawns 3 sequential CLI subprocesses with full container initialization) ISSUES CLOSED: #789
204 lines
7.3 KiB
Python
204 lines
7.3 KiB
Python
"""M4 E2E CLI error-path tests.
|
|
|
|
Tests that the ``agents plan`` CLI subcommands handle error conditions
|
|
gracefully: read-only plans, unavailable actions, missing changesets,
|
|
and empty decision lists.
|
|
|
|
Usage (via dispatcher):
|
|
python robot/helper_m4_e2e_verification.py cli-plan-execute-readonly
|
|
python robot/helper_m4_e2e_verification.py cli-plan-use-not-found
|
|
python robot/helper_m4_e2e_verification.py cli-plan-diff-no-changeset
|
|
python robot/helper_m4_e2e_verification.py cli-plan-tree-empty
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import sys
|
|
from pathlib import Path
|
|
from unittest.mock import MagicMock, patch
|
|
|
|
# Ensure the src directory is on the import path.
|
|
_SRC = str(Path(__file__).resolve().parents[1] / "src")
|
|
if _SRC not in sys.path:
|
|
sys.path.append(_SRC)
|
|
|
|
from helper_m4_e2e_common import ( # noqa: E402
|
|
_ROOT_ULID,
|
|
_assert_exit_code,
|
|
_fail,
|
|
_mock_parent_plan,
|
|
)
|
|
from typer.testing import CliRunner # noqa: E402
|
|
|
|
from cleveragents.application.services.plan_lifecycle_service import ( # noqa: E402
|
|
ActionNotAvailableError,
|
|
)
|
|
from cleveragents.cli.commands.plan import app as plan_app # noqa: E402
|
|
from cleveragents.core.exceptions import PlanError # noqa: E402
|
|
from cleveragents.domain.models.core.action import ActionState # noqa: E402
|
|
from cleveragents.domain.models.core.plan import ( # noqa: E402
|
|
PlanPhase,
|
|
ProcessingState,
|
|
)
|
|
|
|
_runner = CliRunner()
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# cli-plan-execute-readonly (error path)
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def cli_plan_execute_readonly() -> None:
|
|
"""Verify ``plan execute`` aborts on a read-only plan.
|
|
|
|
The ``execute`` command calls ``service.get_plan(plan_id)`` as a
|
|
fail-fast read-only guard. When ``plan.read_only is True``, the
|
|
CLI should print an error and abort without calling ``execute_plan``.
|
|
"""
|
|
mock_service = MagicMock()
|
|
plan = _mock_parent_plan(
|
|
phase=PlanPhase.STRATEGIZE,
|
|
state=ProcessingState.COMPLETE,
|
|
read_only=True,
|
|
)
|
|
mock_service.get_plan.return_value = plan
|
|
|
|
with (
|
|
patch(
|
|
"cleveragents.cli.commands.plan._get_lifecycle_service",
|
|
return_value=mock_service,
|
|
),
|
|
patch(
|
|
"cleveragents.cli.commands.plan._get_plan_executor",
|
|
return_value=MagicMock(),
|
|
),
|
|
):
|
|
result = _runner.invoke(plan_app, ["execute", _ROOT_ULID])
|
|
if result.exit_code == 0:
|
|
_fail(
|
|
f"plan execute should fail for read-only plan\noutput={result.output}"
|
|
)
|
|
output_lower = result.output.lower()
|
|
if "read-only" not in output_lower and "read_only" not in output_lower:
|
|
_fail(f"output should mention read-only\noutput={result.output}")
|
|
|
|
# execute_plan must not have been called
|
|
mock_service.execute_plan.assert_not_called()
|
|
|
|
print("m4-cli-plan-execute-readonly-ok")
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# cli-plan-use-not-found (error path)
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def cli_plan_use_not_found() -> None:
|
|
"""Verify ``plan use`` aborts when the action is not available.
|
|
|
|
Raises ``ActionNotAvailableError`` from ``get_action_by_name``,
|
|
which the CLI catches and prints a user-friendly error message.
|
|
"""
|
|
mock_service = MagicMock()
|
|
mock_service.get_action_by_name.side_effect = ActionNotAvailableError(
|
|
"nonexistent/action", ActionState.ARCHIVED
|
|
)
|
|
|
|
with patch(
|
|
"cleveragents.cli.commands.plan._get_lifecycle_service",
|
|
return_value=mock_service,
|
|
):
|
|
result = _runner.invoke(
|
|
plan_app, ["use", "nonexistent/action", "local/monorepo"]
|
|
)
|
|
if result.exit_code == 0:
|
|
_fail(
|
|
f"plan use should fail for unavailable action\noutput={result.output}"
|
|
)
|
|
output_lower = result.output.lower()
|
|
# Assert command-specific message fragment to avoid accepting
|
|
# unrelated error paths. The production CLI prints:
|
|
# "[red]Action not available:[/red] {e}"
|
|
if "action not available" not in output_lower:
|
|
_fail(
|
|
f"output should contain 'Action not available' "
|
|
f"(got generic or wrong error)\noutput={result.output}"
|
|
)
|
|
|
|
# use_action must not have been called
|
|
mock_service.use_action.assert_not_called()
|
|
|
|
print("m4-cli-plan-use-not-found-ok")
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# cli-plan-diff-no-changeset (error path)
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def cli_plan_diff_no_changeset() -> None:
|
|
"""Verify ``plan diff`` aborts when the plan has no changeset.
|
|
|
|
Raises ``PlanError`` from the apply service ``diff()`` method,
|
|
which the CLI catches and prints ``[red]Diff Error:[/red]``.
|
|
"""
|
|
mock_apply_svc = MagicMock()
|
|
mock_apply_svc.diff.side_effect = PlanError(
|
|
f"Plan {_ROOT_ULID} has no ChangeSet. "
|
|
"Execute phase must complete before viewing diff."
|
|
)
|
|
|
|
with patch(
|
|
"cleveragents.cli.commands.plan._get_apply_service",
|
|
return_value=mock_apply_svc,
|
|
):
|
|
result = _runner.invoke(plan_app, ["diff", _ROOT_ULID])
|
|
if result.exit_code == 0:
|
|
_fail(
|
|
f"plan diff should fail when no changeset exists\n"
|
|
f"output={result.output}"
|
|
)
|
|
output_lower = result.output.lower()
|
|
# Assert command-specific message fragment to avoid accepting
|
|
# unrelated error paths. The production CLI prints:
|
|
# "[red]Diff Error:[/red] {e.message}"
|
|
# where the PlanError message contains "has no ChangeSet".
|
|
if "diff error" not in output_lower and "has no changeset" not in output_lower:
|
|
_fail(
|
|
f"output should contain 'Diff Error' or 'has no ChangeSet' "
|
|
f"(got generic or wrong error)\noutput={result.output}"
|
|
)
|
|
|
|
print("m4-cli-plan-diff-no-changeset-ok")
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# cli-plan-tree-empty (error path)
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def cli_plan_tree_empty() -> None:
|
|
"""Verify ``plan tree`` handles zero decisions gracefully.
|
|
|
|
When ``DecisionService.list_decisions()`` returns an empty list, the
|
|
CLI should print an informational message and exit 0 (not crash).
|
|
"""
|
|
# The production ``tree`` command calls ``container.decision_service()``
|
|
# (not ``container.resolve()``), so the mock must match that API.
|
|
mock_container = MagicMock()
|
|
mock_decision_svc = MagicMock()
|
|
mock_decision_svc.list_decisions.return_value = []
|
|
mock_container.decision_service.return_value = mock_decision_svc
|
|
|
|
with patch(
|
|
"cleveragents.application.container.get_container",
|
|
return_value=mock_container,
|
|
):
|
|
result = _runner.invoke(plan_app, ["tree", _ROOT_ULID])
|
|
_assert_exit_code(result, "plan tree (empty)")
|
|
if "no decisions" not in result.output.lower():
|
|
_fail(f"output should mention 'No decisions'\noutput={result.output}")
|
|
|
|
print("m4-cli-plan-tree-empty-ok")
|