8 Commits

Author	SHA1	Message	Date
HAL9000	80bc9c552d	test: restore and enhance e2e test coverage ... Fix malformed imports in helper_m1_e2e_verification.py and helper_m4_e2e_cli.py where 'from helpers_common import reset_global_state' was incorrectly inserted inside a parenthesized import block, causing Python syntax errors and breaking the lint and unit_tests CI gates. Also fix import ordering in helper_m2_e2e_verification.py, helper_m5_e2e_context.py, helper_m5_e2e_support.py, and helper_m5_e2e_verification.py to satisfy ruff I001 import-sort rules by placing helpers_common imports in the correct group alongside other local robot/ helper imports. ISSUES CLOSED: #8459	2026-05-01 00:59:02 +00:00
HAL9000	7aa50ac489	fix(e2e): restore M5/WF14 tests broken by JSON envelope and stale config ... - Add NO_COLOR=1 to E2E Suite Setup and Test Environment to prevent Rich from injecting ANSI escape codes into JSON output, which breaks Extract JSON From Stdout and JSON assertions in M5/WF14 acceptance tests. - Add reset_global_state() calls to all M1/M2/M4/M5 E2E verification helpers to clear Settings singleton, DI container, provider registry, and SQLAlchemy engine cache between parallel pabot worker runs. - Propagate NO_COLOR=1 at suite-setup level in E2E and integration test resource files so all Run Process subprocesses inherit clean terminal output mode. Root causes: 1. Missing NO_COLOR=1 caused Rich ANSI codes in CLI JSON output, making json.loads() failures in robot Extract JSON From Stdout 2. Stale Settings singleton carried provider keys and DB paths between pabot workers, causing UNIQUE constraint violations and stale config lookups in context policy and server mode E2E tests. Related: PR #9912	2026-05-01 00:59:02 +00:00
freemo	48cff5cfe0	refactor(cli): rename plan lifecycle-list and lifecycle-apply to match specification ... Renames `plan lifecycle-list` to `plan list` and `plan lifecycle-apply` to `plan apply` to align with the specification's canonical command names. Removes legacy V2 plan commands that occupied those names. - Renamed CLI command registrations from lifecycle-list/lifecycle-apply to list/apply - Removed legacy V2 apply and list commands (~200 lines) - Updated apply shortcut in main.py to delegate to v3 lifecycle - Added defensive null check for plan existence in apply command - Updated 63+ test, doc, and benchmark files for consistency Closes #881 Co-authored-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me> Co-committed-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me>	2026-04-02 19:09:04 +00:00
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: #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
aditya	2764fcef5c	fix(actor,preflight,tests): resolve PR #975 review findings and stabilize full-suite coverage runs ... Address review-driven fixes across actor schema, preflight guardrails, docs/examples, and Behave/Robot coverage: unify preflight warning behavior with shared role-warning logic, resolve actor-name to config payloads in production preflight flow, harden response_format validation/coercion edge cases, extract duplicated helper logic, and expand negative-path test coverage. Also fix cross-scenario patcher leakage in step modules to eliminate full-run-only coverage failures.	2026-03-18 06:58:39 +00:00
aditya	26ad778aee	feat(estimation): add estimation actor YAML template and role-aware registration validation ... - Add `role_hint` and `response_format` support to actor schema. - Add non-fatal estimation-role compatibility warnings in actor registration CLI flows. - Add preflight warning path when `estimation` actor is missing `response_format`. - Add `examples/actors/estimator.yaml` and update actor examples documentation/tests. - Update integration helper expectations (m1/m2/m3/m6) for missing provider config in local test env. ISSUES CLOSED: #650	2026-03-18 06:46:14 +00:00
Brent E. Edwards	5e625b22e1	fix(test): convert M1-M6 E2E suites to real subprocess CLI invocations (closes #658) ... Replace CliRunner + unittest.mock.patch with subprocess.run for all 21 CLI-facing test functions across the M1-M6 E2E verification helpers. Application code fixes: - action.py: _get_lifecycle_service() uses container.plan_lifecycle_service() - plan.py: _get_lifecycle_service() uses container.plan_lifecycle_service() - plan.py: three container.resolve(DecisionService) → container.decision_service() Test infrastructure: - New robot/helper_e2e_common.py with shared subprocess utilities (run_cli, setup_workspace with DB migrations, cleanup_workspace) - M1-M4, M6 helpers refactored to use run_cli() with real SQLite DB - M5 unchanged (0 CLI tests, all domain-level) - TDD detection updated to recognise run_cli() as subprocess invocation - Remove @tdd_expected_fail from TDD feature + robot tags - Update 8 Behave step files that mocked container.resolve() to use container.decision_service() / container.plan_lifecycle_service()	2026-03-12 20:42:14 +00:00
brent.edwards	af212bc432	test(e2e): verify M1 success criteria — minimal plan execution flow ... Add Robot Framework end-to-end test suite that verifies the complete M1 success criteria: action creation from YAML, git resource registration, project creation and resource linking, plan use/execute/diff/apply lifecycle, SQLite persistence, ChangeSet from tool invocations, git worktree sandbox isolation, and post-apply commit verification.	2026-02-25 04:49:27 +00:00