Files
cleveragents-core/robot/helper_cli_lifecycle_e2e.py
hurui200320 414abb1396 fix(cli): add missing --yes flag to plan apply command (#1127)
## Summary

Adds the `--yes`/`-y` flag to the `lifecycle-apply` CLI command as required by the specification (`agents plan apply [--yes|-y] <PLAN_ID>`). Without `--yes`, a confirmation prompt now displays before proceeding with the destructive Apply phase. With `--yes`, the apply proceeds immediately without prompting.

Closes #932

## Changes

### Source Code
- **`src/cleveragents/cli/commands/plan.py`**: Added `yes: Annotated[bool, typer.Option("--yes", "-y", help="Skip confirmation prompt")] = False` parameter to `lifecycle_apply_plan`. Added `typer.confirm()` prompt before the apply operation, consistent with the pattern used by `rollback_plan`, `correct_plan`, and other destructive commands.
  - Confirmation prompt text matches spec exactly: `"Apply changes for plan {plan_id}?"` producing `Apply changes for plan <ID>? [y/N]:`.
  - Fixed redundant plan ID display when `pre_plan` is `None` — now shows `"Apply changes for plan X?"` instead of `"Apply plan X (X)?"`.
  - Added `except ValueError` handler consistent with sibling commands `lifecycle_execute_plan` and `_lifecycle_apply_with_id`.
  - Added `except Exception` catch-all handler with `isinstance(e, (typer.Abort, typer.Exit))` re-raise guard, consistent with `lifecycle_execute_plan`.
  - Moved `PlanPhase` and `ProcessingState` imports to module level per CONTRIBUTING.md §Import Guidelines.

### TDD Tag Removal (Bug Fix Workflow)
- **`features/tdd_plan_apply_yes_flag.feature`**: Removed `@tdd_expected_fail` tag (leaving `@tdd_bug` and `@tdd_bug_932` as permanent regression guards).
- **`robot/tdd_plan_apply_yes_flag.robot`**: Removed `tdd_expected_fail` tag (leaving `tdd_bug` and `tdd_bug_932`).

### Test Updates
Updated all existing `lifecycle-apply` invocations across 17 test/benchmark files to pass `--yes`, since the new confirmation prompt would otherwise abort in non-interactive test environments:
- 9 Behave step definition files
- 3 Robot Framework helper scripts
- 2 Robot Framework e2e acceptance tests
- 3 ASV benchmark files (4 invocations: `cli_robot_flow_bench.py` ×2, `m1_sourcecode_smoke_bench.py` ×1, `plan_cli_smoke_bench.py` ×1)

### Confirmation Prompt Tests (New + Strengthened)
- **`features/tdd_plan_apply_yes_flag.feature`**: 5 scenarios total:
  - `lifecycle-apply recognises the --yes long flag` — verifies flag acceptance, prompt suppression, exit code 0, and `apply_plan` was called
  - `lifecycle-apply recognises the -y short flag` — same as above for short flag
  - `lifecycle-apply without --yes prompts for confirmation and user declines` — verifies `"Apply cancelled."` message, `exit_code == 0`, and `apply_plan` was NOT called
  - `lifecycle-apply without --yes prompts for confirmation and user accepts` — verifies prompt appears, `exit_code == 0`, and `apply_plan` was called
  - `lifecycle-apply catches unexpected exceptions cleanly` — verifies `"Unexpected error"` output, no traceback leak, non-zero exit code (exercises the `except Exception` catch-all)
- **`features/steps/tdd_plan_apply_yes_flag_steps.py`**: Refactored step definitions:
  - `_make_mock_plan` uses `PlanPhase` and `ProcessingState` enum types instead of raw strings
  - `_make_mock_plan` uses `datetime.now(tz=UTC)` instead of timezone-naive `datetime.now()`
  - Unified prompt suppression step handles both `--yes` and `-y` via parameterised step pattern
  - Added `When` step for unexpected error scenario with `RuntimeError` side_effect
  - Added `Then` step for non-zero exit code assertion
- **Feature/Robot documentation**: Updated stale descriptions that said "implementation does not accept --yes" to reflect the flag is now implemented.

### Documentation
- **`docs/reference/plan_cli.md`**: Updated `lifecycle-apply` section with:
  - `### Synopsis` heading with code block
  - `### Options` table listing `--yes/-y` and `--format/-f` flags
  - `### Arguments` table listing `PLAN_ID`
  - Matches the style used by other command sections in the same file

## Review Fixes (Cycle 3 — Luis's review)

| ID | Severity | Issue | Resolution |
|----|----------|-------|------------|
| M1 | Medium | `typer.Abort()` on user decline produces exit code 1 and redundant "Aborted." | Changed to `raise typer.Exit(0)` — consistent with `correct_decision` and legacy `apply` |
| M2 | Medium | Missing exit code assertion on decline scenario | Added `And the lifecycle-apply exit code should be 0` to the decline scenario |
| M3 | Medium | Spec compliance: "summary of pending changes" not implemented | Deferred — spec example shows summary *after* confirmation, not before; implementation matches spec. Ticket-vs-spec ambiguity noted. |
| L1 | Low | Missing `except Exception` catch-all handler | Added catch-all matching `lifecycle_execute_plan` pattern; re-raises `typer.Abort`/`typer.Exit` |
| L2 | Low | Documentation description not updated | Expanded description in `plan_cli.md` to explain confirmation prompt and `--yes` |
| L3 | Low | Dead code `is not None` guards | Removed both guards — `get_plan()` raises `NotFoundError`, never returns `None` |
| I1 | Info | Duplicate `PlanPhase` import | Hoisted import to top of `try` block, eliminating duplicate at old line 2087 |
| I2 | Info | `typer.confirm` without explicit `default=False` | Added `default=False` for consistency with sibling commands |
| L4 | Low | No test for `--yes` after positional arg | Not addressed — Typer/Click handles both orderings; low risk |
| L5 | Low | No test for auto-select + interactive prompt | Not addressed — separate concern outside ticket scope |
| I3 | Info | Robot helper only tests flag recognition | By design — noted as informational |

## Review Fixes (Cycle 4 — Self-QA)

| ID | Severity | Issue | Resolution |
|----|----------|-------|------------|
| Major-1 | Major | No test for `except Exception` catch-all handler | Added new scenario `"lifecycle-apply catches unexpected exceptions cleanly"` with `RuntimeError` side_effect; asserts `"Unexpected error"` output, no traceback, non-zero exit |
| Minor-2 | Minor | Missing `ValueError` handler inconsistent with siblings | Added `except ValueError as e:` with `"[red]Execution Error:[/red]"` before catch-all, matching `lifecycle_execute_plan` and `_lifecycle_apply_with_id` |
| Minor-3 | Minor | Flag scenarios don't verify `apply_plan` called | Added `And the lifecycle-apply should have called apply` to both `--yes` and `-y` scenarios |
| Minor-4 | Minor | Stale docstring in Robot helper references `tdd_expected_fail` inversion | Updated to reflect bug is fixed and tests serve as regression guards |
| Minor-5 | Minor | `plan_cli.md` lacks Options table for `lifecycle-apply` | Added Synopsis, Options, and Arguments sections matching sibling command style |
| Nit-6 | Nit | Duplicated step defs for `--yes` vs `-y` prompt suppression | Unified into single parameterised step `"the lifecycle-apply {flag} output should not contain the confirmation prompt"` |
| Nit-7 | Nit | `datetime.now()` timezone-naive | Changed to `datetime.now(tz=UTC)` |
| Nit-8 | Nit | `_make_mock_plan` params use `str` instead of enum types | Changed to `PlanPhase` and `ProcessingState` enum types |

## Review Fixes (Cycle 5 — Jeff's approval note)

| ID | Severity | Issue | Resolution |
|----|----------|-------|------------|
| Import-1 | Minor | `PlanPhase`/`ProcessingState` imports inside function body instead of module level | Moved to module-level import per CONTRIBUTING.md §Import Guidelines |

## Known Limitations / Deferred Items

- **M3: Ticket AC mentions "summary of pending changes"** but the spec example only shows `"Apply changes for plan <ID>? [y/N]: y"` without a change summary. The implementation shows plan ID only (matching the spec), not a change summary. This is a ticket-vs-spec ambiguity; recommend discussing with ticket author.
- **Legacy `apply` command** accepts `--yes` but does not pass it to `_lifecycle_apply_with_id()`. This is a pre-existing issue outside the scope of this ticket.
- **`pre_plan is None` branch** has no explicit test. Pre-existing architectural issue; no action taken.

## Quality Gates

| Gate | Result |
|------|--------|
| `nox -s lint` |  passed |
| `nox -s typecheck` |  passed (0 errors) |
| `nox -s unit_tests` |  passed (471 features, 12,424 scenarios, 0 failures) |
| `nox -s integration_tests` |  passed (1,727 tests, 0 failures) |
| `nox -s e2e_tests` |  passed (41 tests, 0 failures) |
| `nox -s coverage_report` |  passed (≥97% coverage) |

Reviewed-on: cleveragents/cleveragents-core#1127
Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>
Co-authored-by: Rui Hu <rui.hu@cleverthis.com>
Co-committed-by: Rui Hu <rui.hu@cleverthis.com>
2026-03-26 07:50:09 +00:00

350 lines
11 KiB
Python

"""Helper script for cli_lifecycle_e2e.robot end-to-end smoke tests.
Each subcommand is self-contained and prints a sentinel on success.
"""
from __future__ import annotations
import os
import sys
import tempfile
from datetime import datetime
from pathlib import Path
from unittest.mock import MagicMock, patch
# Ensure local source tree is importable
_SRC = str(Path(__file__).resolve().parents[1] / "src")
if _SRC not in sys.path:
sys.path.insert(0, _SRC)
from typer.testing import CliRunner # noqa: E402
from cleveragents.cli.commands.action import app as action_app # noqa: E402
from cleveragents.cli.commands.plan import app as plan_app # noqa: E402
from cleveragents.domain.models.core.action import Action, ActionState # noqa: E402
from cleveragents.domain.models.core.plan import ( # noqa: E402
AutomationProfileProvenance,
AutomationProfileRef,
NamespacedName,
Plan,
PlanIdentity,
PlanPhase,
PlanTimestamps,
ProcessingState,
ProjectLink,
)
runner = CliRunner()
_PLAN_ULID = "01KHDE6WWS2171PWW3GJEBXZ8S"
_VALID_YAML = """\
name: local/e2e-action
description: E2E lifecycle action
strategy_actor: openai/gpt-4
execution_actor: openai/gpt-4
definition_of_done: All e2e tests pass
"""
def _mock_action(name: str = "local/e2e-action") -> Action:
return Action(
namespaced_name=NamespacedName.parse(name),
description="E2E lifecycle action",
long_description=None,
definition_of_done="All e2e tests pass",
strategy_actor="openai/gpt-4",
execution_actor="openai/gpt-4",
state=ActionState.AVAILABLE,
reusable=True,
read_only=False,
created_at=datetime.now(),
updated_at=datetime.now(),
created_by=None,
)
def _mock_plan(
name: str = "local/e2e-plan",
phase: PlanPhase = PlanPhase.STRATEGIZE,
state: ProcessingState = ProcessingState.QUEUED,
project_links: list[ProjectLink] | None = None,
plan_id: str = _PLAN_ULID,
) -> Plan:
now = datetime.now()
return Plan(
identity=PlanIdentity(plan_id=plan_id),
namespaced_name=NamespacedName.parse(name),
description="E2E lifecycle plan",
definition_of_done="Tests pass",
action_name="local/e2e-action",
phase=phase,
processing_state=state,
project_links=project_links or [ProjectLink(project_name="proj-a")],
arguments={"target_coverage": 80},
arguments_order=["target_coverage"],
automation_profile=AutomationProfileRef(
profile_name="trusted",
provenance=AutomationProfileProvenance.PLAN,
),
strategy_actor="openai/gpt-4",
execution_actor="openai/gpt-4",
reusable=True,
read_only=False,
created_by=None,
timestamps=PlanTimestamps(created_at=now, updated_at=now),
)
def _write_yaml(content: str) -> str:
fd, path = tempfile.mkstemp(suffix=".yaml")
with os.fdopen(fd, "w") as fh:
fh.write(content)
return path
# ---------------------------------------------------------------------------
# Subcommands
# ---------------------------------------------------------------------------
def action_create() -> None:
"""Verify action create from config file."""
mock_service = MagicMock()
mock_service.create_action.return_value = _mock_action()
yaml_path = _write_yaml(_VALID_YAML)
try:
with patch(
"cleveragents.cli.commands.action._get_lifecycle_service",
return_value=mock_service,
):
result = runner.invoke(action_app, ["create", "--config", yaml_path])
if result.exit_code == 0:
print("cli-lifecycle-action-create-ok")
else:
print(
f"FAIL: action create returned {result.exit_code}", file=sys.stderr
)
print(result.output, file=sys.stderr)
sys.exit(1)
finally:
os.unlink(yaml_path)
def plan_use() -> None:
"""Verify plan use creates a plan in Strategize phase."""
mock_service = MagicMock()
mock_service.get_action_by_name.return_value = _mock_action()
mock_service.use_action.return_value = _mock_plan()
with patch(
"cleveragents.cli.commands.plan._get_lifecycle_service",
return_value=mock_service,
):
result = runner.invoke(plan_app, ["use", "local/e2e-action", "proj-a"])
if result.exit_code == 0:
print("cli-lifecycle-plan-use-ok")
else:
print(f"FAIL: plan use returned {result.exit_code}", file=sys.stderr)
print(result.output, file=sys.stderr)
sys.exit(1)
def plan_execute() -> None:
"""Verify plan execute transitions to Execute phase."""
mock_service = MagicMock()
mock_service.get_plan.return_value = _mock_plan(
phase=PlanPhase.STRATEGIZE, state=ProcessingState.COMPLETE
)
mock_service.execute_plan.return_value = _mock_plan(
phase=PlanPhase.EXECUTE, state=ProcessingState.QUEUED
)
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", _PLAN_ULID])
if result.exit_code == 0:
print("cli-lifecycle-plan-execute-ok")
else:
print(f"FAIL: plan execute returned {result.exit_code}", file=sys.stderr)
print(result.output, file=sys.stderr)
sys.exit(1)
def plan_apply() -> None:
"""Verify plan lifecycle-apply transitions to Apply phase."""
mock_service = MagicMock()
mock_service.apply_plan.return_value = _mock_plan(
phase=PlanPhase.APPLY, state=ProcessingState.QUEUED
)
with patch(
"cleveragents.cli.commands.plan._get_lifecycle_service",
return_value=mock_service,
):
result = runner.invoke(plan_app, ["lifecycle-apply", "--yes", _PLAN_ULID])
if result.exit_code == 0:
print("cli-lifecycle-plan-apply-ok")
else:
print(f"FAIL: plan apply returned {result.exit_code}", file=sys.stderr)
print(result.output, file=sys.stderr)
sys.exit(1)
def plan_status() -> None:
"""Verify plan status shows plan details."""
mock_service = MagicMock()
mock_service.get_plan.return_value = _mock_plan(
project_links=[ProjectLink(project_name="proj-a", alias="api")]
)
with patch(
"cleveragents.cli.commands.plan._get_lifecycle_service",
return_value=mock_service,
):
result = runner.invoke(plan_app, ["status", _PLAN_ULID])
if result.exit_code == 0:
print("cli-lifecycle-plan-status-ok")
else:
print(f"FAIL: plan status returned {result.exit_code}", file=sys.stderr)
print(result.output, file=sys.stderr)
sys.exit(1)
def plan_cancel() -> None:
"""Verify plan cancel cancels a non-terminal plan."""
mock_service = MagicMock()
mock_service.cancel_plan.return_value = _mock_plan(
phase=PlanPhase.STRATEGIZE, state=ProcessingState.CANCELLED
)
with patch(
"cleveragents.cli.commands.plan._get_lifecycle_service",
return_value=mock_service,
):
result = runner.invoke(
plan_app, ["cancel", _PLAN_ULID, "--reason", "E2E test cancel"]
)
if result.exit_code == 0:
print("cli-lifecycle-plan-cancel-ok")
else:
print(f"FAIL: plan cancel returned {result.exit_code}", file=sys.stderr)
print(result.output, file=sys.stderr)
sys.exit(1)
def plan_list() -> None:
"""Verify lifecycle-list shows plan table."""
mock_service = MagicMock()
mock_service.list_plans.return_value = [
_mock_plan(name="local/plan-a", plan_id="01KHDE6WWS2171PWW3GJEBXZ8A"),
_mock_plan(name="local/plan-b", plan_id="01KHDE6WWS2171PWW3GJEBXZ8B"),
]
with patch(
"cleveragents.cli.commands.plan._get_lifecycle_service",
return_value=mock_service,
):
result = runner.invoke(plan_app, ["lifecycle-list"])
if result.exit_code == 0:
print("cli-lifecycle-plan-list-ok")
else:
print(f"FAIL: plan list returned {result.exit_code}", file=sys.stderr)
print(result.output, file=sys.stderr)
sys.exit(1)
def full_lifecycle() -> None:
"""End-to-end: action create -> plan use -> execute -> apply."""
mock_service = MagicMock()
action = _mock_action()
mock_service.create_action.return_value = action
mock_service.get_action_by_name.return_value = action
plan_strat = _mock_plan(phase=PlanPhase.STRATEGIZE, state=ProcessingState.QUEUED)
plan_exec = _mock_plan(phase=PlanPhase.EXECUTE, state=ProcessingState.QUEUED)
plan_apply = _mock_plan(phase=PlanPhase.APPLY, state=ProcessingState.QUEUED)
mock_service.use_action.return_value = plan_strat
mock_service.execute_plan.return_value = plan_exec
mock_service.get_plan.return_value = _mock_plan(
phase=PlanPhase.STRATEGIZE, state=ProcessingState.COMPLETE
)
mock_service.apply_plan.return_value = plan_apply
yaml_path = _write_yaml(_VALID_YAML)
try:
with (
patch(
"cleveragents.cli.commands.action._get_lifecycle_service",
return_value=mock_service,
),
patch(
"cleveragents.cli.commands.plan._get_lifecycle_service",
return_value=mock_service,
),
patch(
"cleveragents.cli.commands.plan._get_plan_executor",
return_value=MagicMock(),
),
):
# Step 1: Create action
r1 = runner.invoke(action_app, ["create", "--config", yaml_path])
assert r1.exit_code == 0, f"action create failed: {r1.output}"
# Step 2: Plan use
r2 = runner.invoke(plan_app, ["use", "local/e2e-action", "proj-a"])
assert r2.exit_code == 0, f"plan use failed: {r2.output}"
# Step 3: Plan execute
r3 = runner.invoke(plan_app, ["execute", _PLAN_ULID])
assert r3.exit_code == 0, f"plan execute failed: {r3.output}"
# Step 4: Plan apply
r4 = runner.invoke(plan_app, ["lifecycle-apply", "--yes", _PLAN_ULID])
assert r4.exit_code == 0, f"plan apply failed: {r4.output}"
print("cli-lifecycle-full-e2e-ok")
except AssertionError as exc:
print(f"FAIL: {exc}", file=sys.stderr)
sys.exit(1)
finally:
os.unlink(yaml_path)
# ---------------------------------------------------------------------------
# Main dispatcher
# ---------------------------------------------------------------------------
_COMMANDS = {
"action-create": action_create,
"plan-use": plan_use,
"plan-execute": plan_execute,
"plan-apply": plan_apply,
"plan-status": plan_status,
"plan-cancel": plan_cancel,
"plan-list": plan_list,
"full-lifecycle": full_lifecycle,
}
def main() -> None:
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(2)
_COMMANDS[sys.argv[1]]()
if __name__ == "__main__":
main()