HAL9000/temp
0
0
Fork 0
forked from HAL9000/cleveragents-core
Code Pull Requests Activity
1,526 Commits 550 Branches 3 Tags
fix/container-handler-module-missing
Commit Graph

6 Commits

Author SHA1 Message Date
brent.edwards 998aaf25f8 fix(infra): ensure E2E suite setup initializes database before CLI commands 
Centralize E2E initialization in common suite setup so DB-dependent CLI commands no longer rely on per-suite or per-test init workarounds. This also sanitizes suite-home names to prevent invalid path-derived initialization failures in isolated Robot runs.

ISSUES CLOSED: #1023
 2026-04-03 01:43:27 +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
CoreRasurae 007af498b8 refactor(autonomy): rename automation profile task flags to spec names 
Renamed all 11 task-type confidence threshold fields in AutomationProfile
from phase-transition semantics to spec-defined task-type semantics.
Updated all 8 built-in profiles, CLI formatting, YAML schema, services,
and all Behave/Robot tests referencing the old field names.

Post-review fixes:
- Fixed 24 stale old field names in M6 fixture files
  (automation_profiles.json, autonomy_guardrails.json)
- Added model_validator(mode='before') to detect legacy field names
  and raise actionable ValueError with rename mapping
- Added semantic bridge comments in PlanLifecycleService mapping
  task-type thresholds to phase-transition gates
- Added threshold_field to structured log messages for observability
- Restored categorised CLI automation-profile show output to match
  spec (Phase Transitions / Decision Automation / Self-Repair /
  Execution Controls) instead of flat list
- Added missing access_network field to spec show output examples
  (Rich, Plain, JSON, YAML variants)
- Aligned ADR-017 profile fields table to all 11 fields with
  descriptions matching spec Automatable Tasks table
- Aligned automation_profiles.md threshold descriptions with spec
- Added spec section references in phase_reversion.md, error_recovery.md,
  and plan_execute.md for field naming context
- Extended repository roundtrip test to assert all 11 threshold fields
- Fixed benchmark _make_profile() passing safety fields as top-level
  kwargs instead of via SafetyProfile sub-model (incompatible with
  extra="forbid")
- Aligned CLI JSON/YAML output structure for automation-profile show
  with the specification grouped format (phase_transitions,
  decision_automation, self_repair, execution_controls)
- Moved safety boolean fields into the Execution Controls section
  of Rich output per spec examples
- Reverted auto profile description to "Fully automatic except apply"
  per specification (line 16703, line 28406)
- Improved bridge comments in test steps with semantic context for
  threshold-to-gate mappings

ISSUES CLOSED: #902
 2026-03-30 13:18:07 +01: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: 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
hurui200320 83c22b8391 test(e2e): TDD behavioral test proving ACMS indexing pipeline is not wired into CLI (bug #1028) (#1124) 
## Summary

This PR adds a Robot Framework E2E test suite (`robot/e2e/tdd_acms_behavioral_validation.robot`) that proves bug #1028 exists — the ACMS indexing pipeline is not wired into the CLI, so `ContextTierService` starts empty on every invocation.

### Changes

- **New file**: `robot/e2e/tdd_acms_behavioral_validation.robot` — 4 E2E test cases tagged `tdd_expected_fail`, `tdd_bug`, `tdd_bug_1028`, `E2E`
- **Modified**: `robot/e2e/common_e2e.resource` — Extracted shared keywords (`Run CLI`, `Extract JSON From Stdout`, `Link Resource To Project`, `Create Synthetic Codebase`) from both `m5_acceptance.robot` and `tdd_acms_behavioral_validation.robot` to eliminate ~97 lines of duplication. `Create Synthetic Codebase` is parameterized with `project_label`.
- **Modified**: `robot/e2e/m5_acceptance.robot` — Removed duplicated keywords now provided by `common_e2e.resource`.
- **CHANGELOG.md**: Added entry under `## Unreleased` documenting the TDD tests for #1029.

### Test Cases

- **Test 1**: Context Simulate Returns Non-Empty Tier Data — asserts `fragment_count > 0` (fails, proving bug)
- **Test 2**: Context Inspect Shows Indexed Resources — asserts tier metrics total > 0 (fails, proving bug)
- **Test 3**: Budget Enforcement Excludes Oversized Files — asserts `fragment_count > 0` with `max_file_size` policy (fails, proving bug). Includes TODO comment for post-fix exclusion assertion.
- **Test 4**: Large Project Indexes Without Timeout — generates 10K+ files, asserts `fragment_count > 0` (fails, proving bug). Includes explicit developer responsibility note about git-tracking of generated files.

All tests pass CI through result inversion by the `tdd_expected_fail_listener.py` — failing assertions (bug confirmed) are inverted to PASS.

### Documentation & Robustness Improvements

- Suite-level documentation expanded with **known limitation** section explaining `tdd_expected_fail` result inversion scope.
- Suite setup error messages include `(rc=${var.rc}). Check DEBUG logs above.` for debugging consistency with `m5_acceptance.robot`.
- Redundant exit code assertions after `Run CLI` calls removed (Run CLI already validates rc internally).
- `Run CLI` keyword documentation includes API key security notes.
- Budget enforcement test (Test 3) includes `TODO(bugfix/...)` comment for the bug-fix developer.
- Large project test (Test 4) includes explicit NOTE assigning responsibility to the bug-fix developer to evaluate filesystem vs. git-tracked content indexing.

### Review Fix Round

Addressed all findings from Luis's review (review #2691):
- **C1 (CRITICAL)**: Restored the #845 `CorrectionService` changelog entry (50 lines) accidentally deleted during merge conflict resolution.
- **L1 (LOW)**: Extracted ~97 lines of duplicated keywords into `common_e2e.resource` (parameterized `Create Synthetic Codebase`, shared `Run CLI`, `Extract JSON From Stdout`, `Link Resource To Project`).
- **M1 (MEDIUM)**: Strengthened NOTE comment about 10K files not being git-committed — explicit developer MUST responsibility.
- **L3 (LOW)**: Made suite setup error messages verbose with `(rc=...). Check DEBUG logs above.`
- **L2 (LOW)**: Removed redundant exit code assertions after `Run CLI` calls.
- **M2 (MEDIUM)**: Acknowledged — partial assertion is acceptable for TDD capture phase (TODO documents the gap).
- **I1 (INFO)**: Acknowledged — `tdd_expected_fail` masking is documented and mitigated.

### Motivation

Per the Bug Fix Workflow in CONTRIBUTING.md, this TDD issue (#1029) is the prerequisite for bug fix #1028. The tests capture the buggy behavior so that when the fix is implemented, removing the `tdd_expected_fail` tag will cause the tests to pass normally.

### Quality Gates

| Gate | Result |
|------|--------|
| lint | PASS |
| typecheck | PASS |
| unit_tests | PASS |
| integration_tests | PASS |
| e2e_tests | PASS (41 tests, 4 TDD) |
| coverage_report | PASS (>=97%) |

Closes #1029

Reviewed-on: cleveragents/cleveragents-core#1124
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-25 11:13:37 +00:00
Luis Mendes cb583021df test(e2e): E2E acceptance criteria for M6 (v3.5.0) — autonomy hardening 
Implemented Robot Framework E2E test suite for M6 autonomy hardening
acceptance criteria.  Tests exercise the real CleverAgents CLI with zero
mocking, covering session lifecycle, automation profiles, project setup,
plan lifecycle via A2A facade, guard enforcement, and a full autonomy
acceptance flow.  LLM-dependent tests use Skip If No LLM Keys for
graceful degradation when API keys are unavailable.

Hardened shared E2E keywords (common_e2e.resource):
- Safe JSON parsing with rfind-bounded extraction and error wrapping (C1, M1)
- Multi-object fallback: last-line reverse scan for multi-JSON output (M5)
- Moved Safe Parse Json Field to common_e2e.resource for reuse (L1)
- Migrated deprecated Run Keyword If to IF/ELSE blocks (M4)
- Added cwd parameter to Run CleverAgents Command (M6)
- API key protection via inline evaluation instead of RF variables (S1)
- Git return-code assertions in Create Temp Git Repo (L1)
- Removed unused Collections library import (L2)
- Warning log on directory removal failure instead of silent ignore (L6)

Hardened m6_acceptance.robot:
- Force Tags E2E instead of per-test [Tags] (L3)
- Per-test [Teardown] for resource cleanup, including Init test (M2, M9, L5)
- Collision-safe uuid4 hex suffix instead of randint (L7)
- Initialized session_id to EMPTY before test body for safe teardown (L2)
- ELSE branches with WARN log on conditional assertions (M3, M4, M6)
- Guard enforcement checks specific automation-profile fields (M5)
- Strengthened ci assertions with case-sensitive matching and JSON
  field parsing for config get verification (M9)
- Eliminated Python code injection via string interpolation (H2)
- Added assertions to Full Flow Apply Step keyword (H4)
- Removed redundant config set in Guard test (M10)
- Accurate CHANGELOG entry describing actual test scope (C2)

Post-review fixes applied:
- Verify all 8 built-in profiles (manual, review, supervised, cautious,
  trusted, auto, ci, full-auto) instead of 4 (M1)
- Session delete confirms removal via re-list (M2)
- Full Flow Apply Step verifies plan phase transition (M3)
- Plan execute output asserts plan_id presence + phase parsing (M4)
- Safe Parse Json Field gains two-strategy approach: outer-bracket
  extraction then last-line reverse scan fallback (M5)
- Removed redundant automation profile reset in Config test (L1)
- JSON-quoted assertions ("ci", "auto") prevent false-positive
  substring matches on short profile names (L3)

New E2E tests covering remaining acceptance criteria:
- Guard enforcement with custom profile: registers a profile with
  explicit denylist, budget cap, and tool-call limits, then verifies
  all guard fields via automation-profile show (AC-4 / H1)
- Profile precedence resolution: sets global profile to "review",
  creates plan with --automation-profile trusted, asserts plan output
  shows "trusted" not "review" (AC-5 / C2)
- Event queue via plan lifecycle transitions: creates plan, captures
  initial state, executes, verifies state transition proving domain
  event bus delivered and processed events (AC-3 / C1)
- Hierarchical decomposition via plan tree: creates plan with full-auto
  profile, executes, runs plan tree --format json, verifies decision
  nodes and children structure (AC-6 / C3)

Robot Framework uses dots as hierarchy separators in suite names
(e.g. "E2E.M6 Acceptance").  The E2E Suite Setup keyword replaced
spaces with underscores but preserved dots, producing directory names
like "E2E.M6_Acceptance".  The CLI init command derives the project
name from Path.cwd().name, and Project.validate_name() rejects dots
("Name must be alphanumeric with hyphens, underscores, or spaces").

Added a second Replace String call to convert dots to underscores so
the sanitized suite name passes Project name validation.

Also increased subprocess timeouts in m3_e2e_verification.robot (60s->120s)
and m4_e2e_verification.robot (30s->120s) to prevent flaky CI failures from
Python startup overhead.

The plan lifecycle E2E tests call "plan use local/code-review" but
actions are user-defined entities that must be explicitly registered
via "action create --config <yaml>" before use.  Without the action,
plan use failed with "Action 'local/code-review' not found" and all
LLM-dependent tests skipped.

Added action registration to M6 Suite Setup that dynamically selects
the actor matching the available API key (anthropic/claude-sonnet-4
when ANTHROPIC_API_KEY is set, openai/gpt-4o otherwise) and creates
the action YAML inline.

ISSUES CLOSED: #746
 2026-03-17 10:13:23 +00:00