freemo/cleveragents-core
0
0
Fork 0
forked from cleveragents/cleveragents-core
Code Pull Requests Activity
1,409 Commits 352 Branches 3 Tags
fix/1500-impl
Commit Graph

576 Commits

Author SHA1 Message Date
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
freemo 908d5854a3 Merge pull request 'fix(test): remove lingering @tdd_expected_fail tags for closed bugs #797 and #1028' (#1185) from feature/m3-tdd-tag-cleanup into master 2026-04-02 19:08:28 +00:00
freemo 7e4301066c Merge pull request 'refactor(cli): move context commands to actor context subgroup' (#1194) from feature/m4-actor-context-hierarchy into master 2026-04-02 17:53:42 +00:00
freemo 6c94ad9552 Merge pull request 'fix: add --required/--informational flags to validation add CLI' (#1222) from bugfix/m5-validation-required-flag into master 2026-04-02 17:39:19 +00:00
freemo 1d36449a98 fix(cli): remove extra --mode flag from validation attach 
Remove the --mode/-m CLI flag from the validation attach command to align
with the specification, which defines validation mode as an inherent
property of the validation definition set at registration time via
validation add, not as a per-attachment override.

The service layer (ToolRegistryService.attach_validation) no longer
accepts mode as a parameter; instead it reads the mode from the
validation's registered definition. The ToolRegistryRepository
_to_legacy_domain now exposes the mode field so the service can access it.

All tests that passed --mode to the CLI or service have been updated or
removed. The invalid-mode validation scenarios were removed since the
mode is no longer caller-supplied. Coverage remains at 98.7%.

ISSUES CLOSED: #913

Co-authored-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>
Co-committed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>
 2026-04-02 17:07:26 +00:00
freemo dd3770f930 refactor(correction): eliminate redundant fields in CorrectionDryRunReport (#1278) 
Remove three top-level fields from CorrectionDryRunReport that duplicated data already present in the embedded CorrectionImpact object:

- excluded_decisions → now accessed via report.impact.excluded_decisions
- rollback_tier_depth → now accessed via report.impact.rollback_tier_depth
- child_plans_to_rollback → now accessed via report.impact.affected_child_plans

The redundant fields created a divergence risk: both models are frozen=True Pydantic models, so post-construction mutation is impossible, but nothing prevented constructing an instance where the top-level copies disagreed with the impact sub-object.

Approach: Option B (nest-only) — remove the duplicated top-level fields and update all consumers to use the impact object's fields instead.

Closes #1087

Co-authored-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me>
Co-committed-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me>
 2026-04-02 16:59:40 +00:00
brent.edwards e3d3c7926d feat(events): add user_identity field to DomainEvent and propagate through event pipeline (#1257) 
Co-authored-by: Brent Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent Edwards <brent.edwards@cleverthis.com>
 2026-04-02 16:59:07 +00:00
brent.edwards 90215d2a64 test(e2e): update m1_acceptance.robot (#1260) 
Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
 2026-04-02 16:58:37 +00:00
brent.edwards f4432c2759 refactor(cleanup): remove or implement empty stub packages 
Remove four empty stub packages that contained only __init__.py with
__all__ = [] and no functional code, violating CONTRIBUTING.md §Commit
Completeness. All four were audited against docs/specification.md:

- src/cleveragents/runtime/ — Spec defines four layers (Domain,
  Application, Infrastructure, Presentation) with no Runtime Layer.
  Runtime concepts (LSP Runtime, actor runtime) belong in Infrastructure.
- src/cleveragents/domain/repositories/ — Spec mentions repository
  interfaces in Domain but mandates no separate subpackage. Empty with
  no protocols defined; existing models cover the domain.
- src/cleveragents/domain/plans/ — Plan domain models already exist in
  domain/models/planconfig, planfiles, and plansettings. Empty duplicate.
- src/cleveragents/application/workflows/ — Application logic already
  served by application/services/. Empty stub added no value.

Updated test references in features/steps/module_coverage_steps.py,
features/steps/coverage_extras_steps.py, features/architecture.feature,
and robot/architecture.robot to remove the deleted packages from import
verification and architecture validation lists.

ISSUES CLOSED: #948

Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
 2026-04-02 16:58:36 +00:00
brent.edwards 85f8970d00 test(integration): workflow example 8 — cloud infrastructure management (supervised profile) (#1231) 
Robot Framework integration test for Specification Workflow Example 8:
Cloud Infrastructure Management with the supervised automation profile.

Test suite exercises:
- Custom resource type registration (local/terraform-state) via YAML
  fixture with copy_on_write sandbox strategy and CLI args validation
- Custom skill creation (local/terraform-ops) with 3 anonymous tools
  (terraform_plan, terraform_show, cloud_metrics) and skill composition
  via includes (local/file-ops)
- Supervised profile gating: verifies both strategize-to-execute
  (create_tool=1.0) and execute-to-apply (select_tool=1.0) transitions
  require explicit human approval via should_auto_progress()
- Action creation with supervised profile, typed arguments (STRING,
  FLOAT), and invariant propagation to plans via InvariantSource.ACTION
- Mocked infrastructure analysis producing right-sizing recommendations
  for over-provisioned resources while skipping critical instances
- Invariant enforcement blocking modifications to resources tagged
  critical:true while allowing non-critical changes

Fixture files provide deterministic mock Terraform state (3 AWS
resources), CloudWatch metrics (CPU at 12% avg on m5.2xlarge), and
sample HCL configuration.

ISSUES CLOSED: #772

Reviewed-by: freemo (reviewer-pool-1)
Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
 2026-04-02 16:51:46 +00:00
brent.edwards e2c489aba2 test(integration): workflow example 6 — documentation generation from codebase analysis (trusted profile) (#1230) 
Add Robot Framework integration test suite exercising Specification Workflow Example 6: documentation generation from codebase analysis using the trusted automation profile.

The test suite (6 test cases) validates:
- Context policy configuration with strategize/execute view-specific settings and view inheritance resolution
- Budget enforcement filtering ContextFragments by max_file_size and max_total_size limits
- Trusted profile auto-progress behavior (create_tool=0.0 triggers automatic Strategize→Execute; select_tool=1.0 gates Apply)
- Action creation with doc_types/output_dir arguments and 3 invariants matching the spec scenario
- Temp project sandbox with documentation generation into output directory and source-code modification invariant verification
- Full plan lifecycle through trusted profile with argument and invariant preservation

ISSUES CLOSED: #770

Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
 2026-04-02 16:51:39 +00:00
brent.edwards 7e9a45044b fix(session): session create does not persist session for subsequent list 
The CLI `session create` command created the session via SessionService.create()
(which commits via auto_commit=True), then called _facade_dispatch("session.create")
for A2A protocol bookkeeping. The facade handler unconditionally called
svc.create() on a second PersistentSessionService instance (a new Factory
resolution from the DI container with its own engine), creating a duplicate
session in the database.

The fix makes A2aLocalFacade._handle_session_create() idempotent: when a
session_id is already present in the params, it acknowledges the existing
session without creating a new one. The CLI dispatch params were also fixed
to use the correct key name (actor_name instead of actor).

Changes:
- src/cleveragents/a2a/facade.py: Early return in _handle_session_create when
  session_id is already supplied, preventing duplicate session creation.
- src/cleveragents/cli/commands/session.py: Fixed param key from "actor" to
  "actor_name" for consistency with the facade handler.
- features/a2a_facade_wiring.feature: Added idempotency scenario verifying
  that session.create with an existing session_id does not call svc.create().
- features/steps/a2a_facade_wiring_steps.py: Added step asserting mock
  SessionService.create was not called.
- features/tdd_session_create_persist.feature: Removed @tdd_expected_fail tag
  now that the bug is fixed.
- robot/e2e/e2e_session_create_persist.robot: Removed tdd_expected_fail tag,
  updated documentation.
- .semgrep.yml: Excluded wrapping.py from no-exec/no-compile-exec rules
  (pre-existing sandboxed exec usage for tool transforms).

ISSUES CLOSED: #1141

Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
 2026-04-02 16:51:04 +00:00
brent.edwards f002bad4ec test: add TDD bug-capture test for #1152 — budget eviction deletes instead of demotes 
Add Behave BDD scenarios and Robot Framework integration tests that capture bug #1152: _enforce_hot_budget() permanently deletes hot-tier fragments via del self._hot[oldest_id] instead of demoting them to the warm tier. Similarly, evict_lru() permanently deletes via del store[fid].

The specification (§Plan Lifecycle ACMS Actions) describes a downward lifecycle ("Hot context archived to warm") that requires demotion, not destruction.

Two Behave scenarios tagged @tdd_expected_fail @tdd_issue @tdd_issue_1152:
- Budget-evicted fragment should be demoted to warm, not deleted
- evict_lru-evicted fragment should be demoted to warm, not deleted

Two Robot Framework tests mirror the Behave scenarios with a Python helper script for subprocess isolation.

ISSUES CLOSED: #1183

Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
 2026-04-02 16:50:50 +00:00
freemo ba55412ae1 fix(test): remove lingering @tdd_expected_fail tags for closed bugs #797 and #1028 
Remove @tdd_expected_fail tags from test files for bugs #797 (actor list
triggers DB update) and #1028 (ACMS indexing pipeline not wired), both of
which are now closed and merged. The @tdd_bug and @tdd_bug_N tags remain
as permanent regression guards per CONTRIBUTING.md TDD workflow rules.

Files changed:
- features/tdd_actor_list_no_db_update.feature
- robot/tdd_actor_list_no_db_update.robot
- robot/e2e/tdd_acms_behavioral_validation.robot

ISSUES CLOSED: #1182
 2026-04-02 07:05:11 +00:00
brent.edwards 0ae733d01f feat(tui): implement help panel (F1) with context-sensitive help 
Add a dedicated TUI help overlay toggled by F1 and resolve its content from the current prompt mode for main-screen, slash-command, reference, and shell contexts.

Extend Behave and Robot coverage for the new help-panel widget, app wiring, context switching, and toggle behavior.

ISSUES CLOSED: #1013
 2026-04-01 21:51:14 +00:00
hamza.khyari 18b9d61e35 feat(resource): implement ResourceHandler content_hash method 
Add content_hash(resource, *, algorithm='sha256') -> str to the
ResourceHandler protocol and all handler implementations:

- Protocol: new content_hash method on ResourceHandler (protocol.py)
- BaseResourceHandler: default impl hashes file content or directory
  entry names; returns EMPTY_CONTENT_HASH sentinel for missing
  resources
- GitCheckoutHandler: hashes git rev-parse HEAD through the requested
  algorithm for consistent digest format
- FsDirectoryHandler: recursive walk hashing sorted relative paths
  and file contents (content-only, ignores metadata)
- DevcontainerHandler: hashes devcontainer.json config file
- DatabaseResourceHandler: hashes connection string; for SQLite
  file-based DBs, hashes the database file content
- _DefaultHandler: delegates to BaseResourceHandler

EMPTY_CONTENT_HASH sentinel is the SHA-256 of empty input
(e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855).

Hash algorithm is configurable via the algorithm parameter (default
sha256, accepts any hashlib.new()-compatible name).

Behave tests (10 scenarios): sentinel for missing/nonexistent,
determinism, different content produces different hash, fs-directory
recursive hash with change detection, git-checkout hash, configurable
algorithm (sha256 vs sha512), protocol compliance for all 4 handlers.

ISSUES CLOSED: #837
 2026-04-01 13:02:24 +00:00
brent.edwards 34dcf5226f feat(autonomy): automation profile resolution precedence correct (#1196) 
## Summary
- Fix automation profile resolution precedence at plan-creation time to follow plan > action > project > global.
- Narrow exception handling in `_resolve_plan_profile_ref` to catch only `(KeyError, ValueError, OSError)` with warning-level structured logging, replacing bare `except Exception:` per CONTRIBUTING.md §Exception Propagation.
- Fix TDD feature file tag/title inconsistency: align to `@tdd_bug @tdd_bug_1076` / "TDD Bug #1076" referencing the original bug ticket.
- Add BDD scenarios for plan-level profile override and config service error fallback.
- Harden lifecycle and Robot/Behave test paths for auto-progression and parallel CI stability (including pre-seeded DB initialization for `tdd_plan_explain_plan_id` helper).
- Update Robot helper timing/session patterns to reduce false negatives in long-running integration/E2E validations.

## Approach
The automation profile resolution is implemented in `PlanLifecycleService._resolve_plan_profile_ref`, which evaluates the four-level precedence chain (plan > action > project > global) at `use_action()` time. The resolved profile is stored as an `AutomationProfileRef` on the Plan with provenance tracking. The `ConfigService` is used for project-scoped and global config lookups, with narrowed exception handling that logs warnings on failure and falls back to the settings default.

## Validation
- `nox -s lint` ✅
- `nox -s typecheck` ✅
- `nox -s unit_tests` ✅
- `nox -s coverage_report` ✅ (97%)

## Review Fix Round (Review #2963)
1. **Bare `except Exception:` → narrowed to `(KeyError, ValueError, OSError)` with warning logging** — addresses CONTRIBUTING.md §Exception Propagation violation
2. **Tag inconsistency fixed** — `@tdd_bug @tdd_bug_1076` with title "TDD Bug #1076"
3. **Added 2 new BDD scenarios** — plan-level override and config error fallback
4. **Rebased onto latest master** (532ea100)

Closes #854

Reviewed-on: cleveragents/cleveragents-core#1196
Co-authored-by: Brent Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent Edwards <brent.edwards@cleverthis.com>
 2026-04-01 04:27:09 +00:00
brent.edwards 81c141716b test(integration): workflow example 2 — automated test generation for a module (trusted profile) (#1176) 
## Summary
- Refactors WF02 integration helper into focused modules to satisfy maintainability guidelines and keep command responsibilities isolated.
- Moves WF02 artifact assertions onto lifecycle-produced change artifacts and verifies user-facing `plan artifacts` flow with mocked providers.
- Reworks WF02 coverage-gate assertions to run through validation registration/attachment/execution lifecycle and apply-gate outcomes.

## What changed in this fix round (cycle-6)
- **CI must-fix — trusted profile lifecycle integration failure:**
  - Fixed failing integration test `WF02 Trusted Profile Auto Runs Strategize And Execute` caused by legacy AutomationProfile fields that no longer exist.
  - Updated WF02 trusted-profile assertions from removed legacy fields (`auto_strategize`, `auto_execute`, `auto_apply`) to current spec-aligned fields (`decompose_task`, `create_tool`, `select_tool`).
  - Key module: `robot/wf02_test_generation_commands.py` (`wf02_trusted_lifecycle`).

## Prior fix rounds retained
- **P1 must-fix — validation payload assertions strengthened:**
  - Added explicit assertions for validation payload correctness in both failing and passing branches.
  - Assertions now verify:
    - validation identity (`local/auth-coverage` on `local/api-service-repo`)
    - required-mode semantics (`mode=required`, required pass/fail counters, `all_required_passed`)
    - `passed` boolean for each branch
    - measured-vs-target semantics (`measured_coverage` compared against `target`).
  - Key module: `robot/wf02_test_generation_validation.py`.

- **P2 should-fix — artifact membership assertions tightened:**
  - Artifact assertions now enforce exact expected membership and count instead of permissive subset checks.
  - Persisted changeset assertions were aligned to the same strict set/count checks.
  - Key module: `robot/wf02_test_generation_artifacts.py`.

- **P2 should-fix — removed unused helper:**
  - Deleted unused `_bind_changeset_to_plan` helper to eliminate dead code.
  - Key module: `robot/wf02_test_generation_artifacts.py`.

- **P2 should-fix — concrete typing:**
  - Replaced `_setup_validation_db` return type from `tuple[Any, Any]` to concrete typed return (`Callable[[], _NoCloseSession]`, `Session`).
  - Key module: `robot/wf02_test_generation_validation.py`.

## Quality gates
- `nox -e lint` ✅
- `nox -e typecheck` ✅
- `nox -e unit_tests` ✅
- `nox -e integration_tests` ✅
- `nox -e e2e_tests` ✅
- `nox -e coverage_report` ✅ (coverage: **98.74%**)

## Scope / limitations
- No deferred items identified in cycle-6.

Closes #766

Reviewed-on: cleveragents/cleveragents-core#1176
Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>
Co-authored-by: Brent Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent Edwards <brent.edwards@cleverthis.com>
 2026-04-01 03:30:00 +00:00
brent.edwards 91cc0b1446 test: add TDD bug-capture test for #1025 — plan correct auto-resolve (#1172) 
## Summary
- add Behave `@tdd_expected_fail` bug-capture scenarios for `plan correct` auto-resolve without `--plan`
- add shared mock fixtures and Robot integration coverage for isolated-container divergence reproducing bug #1025
- add ASV benchmark coverage for active-plan filtering and document the new TDD capture in changelog

## Testing
- nox -s unit_tests -- features/tdd_plan_correct_auto_resolve.feature
- nox -s integration_tests -- --include tdd_bug_1025
- nox -s lint
- nox -s typecheck
- nox -s coverage_report
- nox

Closes #1035

Reviewed-on: cleveragents/cleveragents-core#1172
Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>
Co-authored-by: Brent Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent Edwards <brent.edwards@cleverthis.com>
 2026-04-01 00:40:46 +00:00
brent.edwards 01b6eb1804 feat(autonomy): parallel execution scales to 10+ concurrent subplans (#1201) 
## Summary

Add M6 parallel-scaling coverage for 10+ concurrent subplans:

- **15-subplan parallel scenario** with explicit peak-concurrency bound checks (`max_parallel=10`) and thread-safe concurrency tracking via `_build_executor()`.
- **Deep hierarchical decomposition** coverage (4+ levels) with adjusted leaf condition that only stops early when hitting `max_depth` or when the workset is trivially small (`min_files_per_subplan`).
- **Non-progress guard** in `_build_hierarchy` to prevent pathological recursion when clustering cannot meaningfully split the file set.
- **Small-project regression test** (< 50 files) verifying decomposition depth does not increase unexpectedly with the relaxed leaf condition.
- **ASV benchmark** for 15-subplan parallel execution with `max_parallel=10` to track scaling behavior.

### Removed from this PR

The `_build_hierarchy` child-linkage correctness fix (returning `node_id` from recursive calls instead of using `nodes[-1].node_id`) has been **removed** per review feedback — it is a separate bug fix and will be submitted as an independent issue/PR per CONTRIBUTING.md §Atomic Commits.

## Approach

- **Concurrency tracking:** The `_build_executor()` closure in step definitions detects `context.concurrency_counter` / `context.concurrency_lock` and performs thread-safe peak tracking in a try/finally block.
- **Leaf condition:** Replaced the `max_files_per_subplan` / `max_tokens_per_subplan` leaf check with a `min_files_per_subplan` check to allow deeper decomposition for large projects. Added a non-progress guard so clustering that cannot split the file set terminates immediately rather than recursing to `max_depth`.
- **Deterministic IDs:** `_ids_for_count()` preserves legacy fixed IDs for the first 5 subplans and generates additional deterministic IDs for scale scenarios.

## Validation

### Passing
- `nox -s lint` — all checks passed
- `nox -s typecheck` — 0 errors, 0 warnings
- `nox -s unit_tests` — 12,988 scenarios passed, 0 failed
- `nox -s coverage_report` — 97% (passes `--fail-under=97`)

Closes #855

Reviewed-on: cleveragents/cleveragents-core#1201
Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
 2026-03-31 23:57:39 +00:00
hamza.khyari 3f66221938 test(integration): workflow example 5 — database schema migration with safety nets (review profile) 
Robot Framework integration test suite for Specification Workflow Example 5:
Database Schema Migration with Safety Nets.  Exercises the review automation
profile with custom resource types, custom skills with database tools,
phased child plan execution, checkpointing, and rollback.

- 7 test cases covering: custom resource type registration, review profile
  behavior, custom skill with 3 database tools, action creation with
  review profile, checkpoint/rollback via managers, 5-phase sequential
  SubplanService.spawn with fail-fast, and plan lifecycle through
  strategize-to-execute
- Uses mocked LLM providers (CLEVERAGENTS_TESTING_USE_MOCK_AI=true)
- All Run Process calls have timeout=60s on_timeout=kill
- Fix environment-dependent test failures: mock shutil.disk_usage in
  disk space and diagnostics --check Behave steps so tests pass
  regardless of CI runner disk space; Robot diagnostics test tolerates
  disk-space errors

ISSUES CLOSED: #769
 2026-03-31 11:03:20 +00:00
brent.edwards a5cc81354d fix: add --required/--informational flags to validation add CLI 
Add --required and --informational as mutually exclusive boolean options
to the `agents validation add` CLI command. When specified, they override
the `mode` field from the YAML config file. This aligns the CLI with the
specification (specification.md line 22339) which states the validation
mode can be set "via --required/--informational on agents validation add".

The fix resolves the spec contradiction by:
- Implementing the flags in the CLI (per Core Concepts > Validation Mode)
- Updating the formal CLI reference to include the new flags
- Adding an exception to Design Principle #3 for validation add
- Removing the spurious positional name argument from the walkthrough

TDD tests from #1102 now run normally with @tdd_expected_fail removed.
New edge-case tests cover mutual exclusivity (both flags rejected).

Also excludes tool/wrapping.py from semgrep exec/compile rules since
that module intentionally uses exec() in a controlled sandbox for the
validation transform feature.

ISSUES CLOSED: #1038
 2026-03-31 07:42:06 +00:00
brent.edwards 84b0c10dbf fix(cli): plan correct active-plan resolution in isolated environments (#1184) 
## Summary
- tighten `plan correct` active-plan fallback so it only runs for isolated `CLEVERAGENTS_HOME` mismatch cases and never when explicit DB env overrides are configured
- narrow fallback exception handling in `_resolve_active_plan_id()` to expected DB/path/service failures; unexpected errors now surface instead of being silently swallowed
- add BDD regression coverage for both safeguards in `features/consolidated_plan_misc.feature` + `features/steps/plan_cli_legacy_r2_steps.py`

## Validation
- `nox -e lint`: PASS
- `nox -e typecheck`: PASS
- `nox -e unit_tests`: PASS
- `nox -e integration_tests`: FAIL in current branch baseline (29 failing Robot integration tests in this environment)
- `nox -e e2e_tests`: FAIL in current branch baseline (45 failing E2E tests in this environment)
- `nox -e coverage_report`: PASS (97%)

Closes #1025

Reviewed-on: cleveragents/cleveragents-core#1184
Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>
Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
 2026-03-31 01:49:16 +00:00
CoreRasurae a4a6b061a6 fix(db): align v3_plans schema with specification DDL 
Aligned v3_plans table with specification DDL:

1. Added effective_profile_snapshot column (TEXT NOT NULL) for
   storing frozen JSON snapshot of automation profile at plan
   creation time.  Added Pydantic field_validator ensuring the
   value is well-formed JSON.  Validator catches RecursionError
   for deeply nested JSON, consistent with automation_profile
   deserialization hardening.  Validator error message uses
   length-only to avoid potential information disclosure.
   Documented that the default "{}" exists for backward
   compatibility; new plans should explicitly set the snapshot.

2. Made root_plan_id NOT NULL — root plans self-reference their
   own plan_id, child plans reference the root ancestor.  Added
   explicit ondelete="RESTRICT" FK policy for consistency with
   other FKs in the model.  Documented known FK policy drift
   between ORM model (RESTRICT) and migrated databases (retained
   SET NULL) in the migration; data integrity is preserved by
   the NOT NULL constraint regardless.  Moved root_plan_id
   self-reference resolution into a PlanIdentity model_validator
   so the domain model is consistent with the DB NOT NULL
   constraint before and after persistence (previously the
   resolution only happened in from_domain(), creating an
   asymmetry where root_plan_id was None in-memory but non-null
   after round-tripping through the database).

3. Made automation_profile NOT NULL with default "balanced".

4. Documented intentional deviation: phase default is "action"
   (code) vs "strategize" (spec) because the Action phase was
   added as a pre-Strategize setup step.

5. Created Alembic migration with backfill logic for existing
   rows.  Root-ancestor backfill uses level-by-level propagation
   with a parent-readiness guard to correctly resolve plans at
   arbitrary hierarchy depth (3+ levels).  Added safety bound
   (max 100 iterations) with logged error on exhaustion to guard
   against cycles in parent_plan_id.  Merged batch_alter_table
   operations to avoid redundant full-table copies in SQLite
   batch mode.  Migration backfill also handles empty-string
   automation_profile values.  Documented downgrade limitation
   (backfill is not reversible).  Orphan-row fallback now logs
   affected row count at WARNING level.  Migration cycle-detection
   now logs affected plan_id values before the orphan fallback
   overwrites them.  All migration SQL uses sa.text() for
   consistency with SQLAlchemy best practices.

6. Hardened automation_profile deserialization in to_domain() to
   catch ValueError (invalid StrEnum provenance), Pydantic
   ValidationError, and RecursionError (deeply nested JSON) in
   addition to JSONDecodeError and KeyError, preventing
   unreadable plans from corrupted DB rows.  Applied the same
   defensive deserialization pattern to effective_profile_snapshot
   in to_domain(): corrupted JSON falls back to '{}' with a
   WARNING log instead of crashing the read path.  Added TypeError
   to the effective_profile_snapshot exception list in to_domain()
   for consistency with the Pydantic validator.  Logging of
   unparseable values uses length only to avoid potential
   information disclosure.

7. Used explicit None check (is not None) instead of truthiness
   for root_plan_id resolution in from_domain(), for
   effective_profile_snapshot in to_domain(), and in
   _serialize_automation_profile() for consistency.

8. Documented intentional column naming conventions vs spec DDL
   (e.g. automation_profile vs automation_profile_name, *_actor
   vs *_actor_name, processing_state vs state, v3_plans vs
   plans).  Documented the semantic difference: automation_profile
   stores either a bare name or structured JSON with provenance,
   whereas the spec automation_profile_name stores a plain name.

9. Fixed benchmark plan constructors
   (plan_phase_migration_bench.py) that were missing the now-
   required root_plan_id and effective_profile_snapshot fields.

10. Replaced defensive getattr() with direct attribute access for
    effective_profile_snapshot in from_domain() and update(),
    since the field is now defined on the Plan domain model.

11. Fixed Any type annotation in test helper _make_plan() to use
    AutomationProfileRef | None for proper type safety.

12. Added BDD scenarios for PlanIdentity self-reference
    resolution, NULL effective_profile_snapshot constraint
    enforcement, valid-JSON-missing-profile_name-key
    deserialization, invalid-JSON and empty-string snapshot
    rejection by Pydantic validator, and corrupted
    effective_profile_snapshot DB fallback in to_domain().

13. Extracted default automation profile name to a module-level
    constant (DEFAULT_AUTOMATION_PROFILE) to reduce sentinel
    duplication across models.py and repositories.py.

14. Centralised automation-profile serialisation into
    LifecyclePlanModel._serialize_automation_profile() to
    eliminate duplication between from_domain() and
    LifecyclePlanRepository.update().

15. Fixed to_domain() root_plan_id type cast from str | None
    to str, reflecting the NOT NULL column constraint.

16. Added PlanIdentity model_validator that resolves None
    root_plan_id to plan_id at domain construction time, ensuring
    the domain model honours the spec DDL NOT NULL constraint
    regardless of persistence state.  Simplified from_domain()
    root resolution accordingly.

ISSUES CLOSED: #921
 2026-03-30 23:40:36 +01:00
freemo 7b3fcaf466 feat(config): implement three-scope config resolution with local config file support 
Implement three-scope configuration resolution (local > project > global)
with deep merge. Add ConfigScope enum, project-root discovery, and
config.local.toml file support.

Changes:
- Add LOCAL to ConfigLevel enum and new ConfigScope enum (GLOBAL, PROJECT, LOCAL)
- Implement discover_project_root() walking up from CWD for cleveragents.toml/.cleveragents
- Implement config.local.toml file loading in ConfigService
- Implement three-scope deep merge via _deep_merge() helper
- Add read_project_config(), read_local_config(), read_merged_config() methods
- Add write_scoped_config() for writing to any scope file
- Update set_value() to accept optional scope parameter
- Update resolve() with six-level precedence chain (CLI > env > local > project > global > default)
- Update CLI config set with --scope flag (global/project/local)
- Add config.local.toml to .gitignore and DEFAULT_IGNORE_PATTERNS
- Add Behave BDD scenarios for three-scope resolution, deep merge, and project-root discovery

ISSUES CLOSED: #937
 2026-03-30 21:48:01 +00:00
freemo 603dc841b0 refactor(cli): move context commands to actor context subgroup 
Register context as a sub-app of the actor Typer group so commands are
accessible under the canonical specification path `agents actor context`.
The top-level `agents context` group remains as a deprecated alias
(with Typer deprecated=True) for one release cycle.

Key changes:
- context.app registered under actor.app in main.py
- `rm` subcommand renamed to `remove`; `rm` kept as hidden deprecated
  alias that prints a deprecation warning before delegating
- Programmatic rm_command() deprecated in favor of remove_command()
- Help text updated to reference canonical `agents actor context` paths
- Behave and Robot tests updated to use canonical paths and renamed
  subcommand; assertion for context help lowercased for flexibility
- All nox sessions pass: lint, typecheck, unit_tests (495 features,
  12731 scenarios), coverage at 97%

ISSUES CLOSED: #888
 2026-03-30 21:39:26 +00:00
freemo 38e05ac45a feat(correction): wire checkpoint rollback into correction service revert flow 
Implement the full correction-checkpoint rollback pipeline:

- Workspace snapshots: CheckpointService.create_workspace_snapshot()
  creates diff-based checkpoints before decision execution, storing
  only changed file paths in metadata.extra["diff_paths"]

- CorrectionService.revert_decisions(): new high-level entry point
  that creates a correction, computes impact, invokes checkpoint
  rollback, and archives artifacts in a single call

- Physical artifact archival: CheckpointService.archive_artifacts()
  moves files to .cleveragents/archived_artifacts/ instead of just
  flagging metadata. CorrectionService._archive_decision_artifacts()
  delegates to this during revert execution

- Selective rollback: CheckpointService.selective_rollback() wraps
  rollback_to_checkpoint with atomic semantics — captures HEAD before
  rollback and recovers on failure

- Diff-based storage: _compute_diff_snapshot() computes changed paths
  between checkpoints via git diff; snapshots store diff manifest and
  SHA-256 hash in metadata

- CLI: plan rollback now accepts --to-checkpoint <id> in addition to
  the positional checkpoint_id argument; uses selective_rollback for
  atomic execution

- DI wiring: Container now injects checkpoint_service into
  CorrectionService; CLI correct command uses container-provided
  service instead of ad-hoc instance (fixes bug #986)

- Checkpoint model: pre_decision added to allowed checkpoint_type
  values

- TDD: Removed @tdd_expected_fail from wiring test feature since the
  DI bug is now fixed

ISSUES CLOSED: #943
 2026-03-30 17:06:49 -04:00
freemo 297823c291 feat(cli): add actor context remove, export, and import commands 
Implement three new actor context subcommands as specified in the v3
CLI specification:

- `agents actor context remove [--yes|-y] (--all|-a|<NAME>)`:
  Remove a named context or all contexts with interactive confirmation.
  Supports --all for bulk removal and --yes to skip prompts.

- `agents actor context export (--output|-o) <FILE> <NAME>`:
  Export a named context to JSON or YAML (detected from file extension).
  Includes sha256 integrity checksum in output metadata.

- `agents actor context import [--update] (--input|-i) <FILE> [<NAME>]`:
  Import a context from JSON or YAML. Name is inferred from file metadata
  if omitted. Refuses to overwrite existing contexts unless --update is
  specified.

All commands:
- Support 6 output formats (json, yaml, plain, table, rich, color) via
  the shared format_output framework
- Use ContextManager for persistence (messages, metadata, state,
  global_context)
- Follow existing error handling patterns (typer.Exit for user errors)
- Are wired as `agents actor context` subcommand group via add_typer

New files:
- src/cleveragents/cli/commands/actor_context.py (command module)
- features/actor_context_cmds.feature (Behave BDD scenarios)
- features/steps/actor_context_cmds_steps.py (BDD step definitions)
- robot/actor_context_export_import.robot (Robot integration test)

ISSUES CLOSED: #869
 2026-03-30 19:35:23 +00:00
freemo a9465c4865 fix(cli): derive actor name from config file instead of positional argument 
Remove the <NAME> positional argument from `actor add` command. The actor
name is now derived from the `name` field inside the config YAML file, per
the specification: `agents actor add (--config|-c) <FILE> [--update]`.
Updated all BDD and integration tests to pass name via config file.

ISSUES CLOSED: #914
 2026-03-30 18:27:59 +00:00
freemo 1c1f477208 fix(cli): align output envelope with specification structure 
Align the structured output envelope (JSON/YAML modes) with the specification:
add `status` field, rename `elements` to `data`, rename `metadata` to
`messages`, and fix the fallback chain to include the table step
(rich→table→color→plain).

ISSUES CLOSED: #884
 2026-03-30 17:48:48 +00:00
hamza.khyari 49015c6bee fix(cli): implement --execution-env-priority on project context set 
Add --execution-env-priority flag to 'project context set' command,
enabling project-level execution environment priority per spec WF17.

- Add execution_env_priority field to ContextConfig domain model
- Validate flag value against ExecutionEnvPriority enum (fallback/override)
- Persist in context_policy_json, preserving existing execution_environment
- Display in 'project context show' Execution Environment section
- Merge with existing blob to avoid overwriting previously set fields

Tests: 9 Behave scenarios, 26 steps.

ISSUES CLOSED: #1079
 2026-03-30 14:33:28 +00:00
hamza.khyari 2370e19da8 feat(resource): add container infrastructure resource types 
Add 7 container infrastructure resource types per ADR-039:
container-runtime, container-image, container-mount, container-exec-env,
container-port, container-volume, container-network.

- YAML configs under examples/resource-types/
- Bootstrap registration via _resource_registry_container.py
- Updated container-instance parent_types (container-runtime, container-image)
  and child_types (container-mount, container-exec-env, container-port)
- container-runtime is top-level with auto-discovery rules (scan_depth: 1)
- container-mount/exec-env/port inherit snapshot sandbox from instance
- container-volume is user-addable with snapshot sandbox
- container-network is read-only, no sandbox
- Handler references are forward declarations (ADR-039)
- 33 Behave BDD scenarios, 4 Robot integration tests
- Updated BUILTIN_NAMES, service docstring, CHANGELOG

ISSUES CLOSED: #831
 2026-03-30 13:44:54 +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
hamza.khyari c97ec273a9 feat(lsp): add missing LspServerConfig model fields 
Add specification-required fields to LspServerConfig:

- description: str (max_length=1000, default='')
- transport: LspTransport enum (stdio/tcp/pipe, default=stdio)
- initialization: dict[str, Any] (LSP initializationOptions, default={})
- workspace_settings: dict[str, Any] (workspace/didChangeConfiguration, default={})

All fields have defaults for backward compatibility with existing
configs. LspTransport enum exported from lsp package.

Tests: 17 Behave scenarios, 5 Robot integration tests.
Existing LSP tests: 250/250 pass unchanged.

ISSUES CLOSED: #835
 2026-03-30 11:51:27 +00:00
freemo 9cace15d6e test(e2e): workflow example 18 — container with remote repo clone (trusted profile) 
E2E test for Specification Workflow Example 18: Container with Remote
Repo Clone using the trusted automation profile.  Exercises the full
container-instance workflow per the spec:

- Registers a container-instance resource with --clone-into flag (new
  CLI flag added to resource add for REPO_URL:CONTAINER_PATH).
- Two-step project creation: project create + project link-resource.
- Dynamic LLM actor selection (Anthropic/OpenAI) matching available
  API keys.
- Plan use with --execution-environment container,
  --execution-env-priority fallback, and --automation-profile trusted.
- Full plan lifecycle: strategize, execute, diff, apply, status.
- Positive assertions: non-empty output, plan ID presence, terminal
  state verification, container/clone evidence logging.
- Negative assertions: Traceback and INTERNAL error checks.
- ULID regex uses Crockford base32 character set.
- Unique suffix for resource/project names (parallel CI safe).
- Skip If No LLM Keys guard for graceful degradation.
- CHANGELOG.md updated.

ISSUES CLOSED: #764
 2026-03-30 19:17:41 +08:00
hurui200320 b6c3169634 test(e2e): workflow example 16 — devcontainer-driven development (supervised profile) 
Implements E2E test for devcontainer-driven development workflow.
Tests project setup with .devcontainer config, resource registration,
and full plan lifecycle with supervised automation profile.

Changes from Cycle 5 review:

1. Replaced custom WF16_ALLOW_XFAIL_SKIP escape hatch with the project
   standard tdd_expected_fail tag system. Test is now tagged
   [Tags] tdd_expected_fail tdd_bug tdd_bug_762 and the listener
   inverts failures in CI until devcontainer features are fully wired.

2. Added diagnostic WF16 Test Teardown keyword that captures plan
   status JSON on failure, mirroring the WF05 teardown pattern.

3. Added stderr checks to all Traceback/INTERNAL guard assertions
   so tracebacks appearing only in stderr are also detected.

4. Rebased branch onto latest master (e4c01492).

5. Documented timeout budget: added comment explaining theoretical
   worst-case (~46min) vs realistic execution under the 35min limit.

6. Added fail-fast warning in AC-3 check noting that AC-4, AC-5, and
   AC-6 are dependent on AC-3 and are likely to also fail.

7. Reordered plan apply arguments to --yes --format json <plan_id>
   for spec consistency.

8. Decomposed long ready_for_apply Evaluate expression into three
   intermediate booleans (is_apply_phase, is_execute_complete,
   is_already_applied) for readability.

9. Added shared_session.close() calls in resource_dag.robot before
   each final print() to ensure explicit session cleanup.

10. Cleaned up stale xfail/skip classification references in log
    messages.

Changes from earlier cycles (3-4) retained in this commit:

- Terminal state validation, AC output combination, JSON format flags,
  suite-level init, Force Tags, automation profile fallback, ULID regex
  IGNORECASE, AC-5 Evaluate decomposition, defensive re-check pattern.

ISSUES CLOSED: #762
 2026-03-30 18:45:10 +08:00
aditya 4e64544aae feat(acms): projects with 10,000+ files index without timeout 
Add explicit indexing timeout bounds and propagate timeout_seconds through the repo indexing service, utility walker, and CLI entrypoint so large repositories fail predictably instead of hanging.

Add 10K-scale Behave/Robot verification and a dedicated benchmark path to validate that large-project indexing completes, persists status correctly, and remains measurable for regressions.

Address review feedback by splitting oversized Behave and Robot support files into focused modules, trimming repo_indexing_service.py back under the 500-line limit, and rebasing the branch onto current master.

ISSUES CLOSED: #851

# Conflicts:
#	CHANGELOG.md
#	robot/helper_m5_e2e_verification.py
 2026-03-30 10:18:19 +00:00
aditya f0442e835d feat(acms): plan execution leverages ACMS context for LLM calls 
Integrate ACMS execute-phase context assembly into LLMExecuteActor and inject assembled context into execute prompts with resilient fallback when assembly fails or returns empty output.

Wire the plan CLI executor to use an ACMS-backed execute context assembler, add Behave coverage for context injection/fallback/empty context, and extend Robot M5 verification helpers to assert execute-phase ACMS context usage.

ISSUES CLOSED: #850
 2026-03-30 09:21:51 +00:00
aditya ef8f9640fa feat(acms): context analysis produces meaningful summaries 
Add phase-wise context analysis summaries for strategize, execute, and apply, and integrate them into `agents project context inspect` and `agents project context simulate` with per-phase resource counts, size and token utilization, and narrowing diagnostics.\n\nAdd Behave and Robot coverage for empty, single-resource, multi-resource, and budget-constrained context analysis, and tighten the M5 smoke coverage after review feedback with exact summary assertions and a missing-path regression scenario for `agents context add`.\n\nISSUES CLOSED: #849
 2026-03-30 08:42:48 +00:00
hurui200320 5ddc04bcc7 test(e2e): workflow example 17 — explicit container with directory mount (trusted profile) 
Implements E2E test for Specification WF17: Explicit Container with
Directory Mount.  The test exercises the full workflow from the spec:

- Registers git-checkout resource and container-instance resource
  (with --image flag; --mount not yet available on CLI)
- Creates project, links both resources via project link-resource
- Sets execution environment to container via project context set
- Uses --execution-env-priority override on plan use (per ADR-043)
- Creates action with dynamically selected actor (Anthropic or OpenAI)
- Runs full plan lifecycle: use → strategize → execute → diff → apply → status
- Uses trusted automation profile for auto-execution of tool invocations

Quality improvements over initial implementation:
- Custom suite setup with workspace init (agents init --force --yes)
- UUID-based unique suffix for resource/project names (CI parallelism)
- Dynamic actor selection matching available API keys (M6 pattern)
- Skip If No LLM Keys guard for graceful degradation
- Crockford Base32 ULID regex ([0-9A-HJ-NP-Z]{26})
- Checked return codes on all git subprocess calls
- INTERNAL error checks alongside Traceback checks
- --format json on all CLI commands for deterministic output
- Domain-specific assertions for plan output verification
- Diff output non-empty assertion
- Security note documenting trusted profile auto-execution decision
- CHANGELOG entry added

ISSUES CLOSED: #763
 2026-03-30 08:05:41 +00:00
hurui200320 d24959e961 test(e2e): workflow example 12 — large-scale hierarchical feature implementation (supervised profile) 
Implements E2E test for Workflow Example 12: large-scale hierarchical
feature implementation with the supervised automation profile.

Key test behaviors:
- Sets up 4 projects (protos, api, worker, frontend) with per-project
  invariants, matching the specification.
- Creates action YAML with spec-required fields: estimation_actor,
  invariant_actor, automation_profile: cautious, and action-level
  invariants.
- Passes all 4 projects to plan use (spec Step 3).
- Dynamically selects actor based on available API key (Anthropic or
  OpenAI) for flexible CI/local execution.
- Uses Skip If No LLM Keys for graceful degradation in keyless CI.
- Verifies hierarchical decomposition via plan tree (decision_id count,
  children field check).
- Selects a non-root decision for plan correct --mode append to
  exercise targeted correction rather than root invalidation.
- Verifies correction acknowledgment via output content assertions.
- Verifies phased lifecycle-apply with --yes flag and phase field
  parsing.
- Final status uses --format json with Safe Parse Json Field to verify
  terminal state (phase and processing_state populated).
- UUID-suffixed names prevent UNIQUE constraint collisions in parallel CI.
- Crockford Base32 ULID regex (excludes I, L, O, U) consistent with
  m1_acceptance.robot.
- All commands check for Traceback and INTERNAL error markers.
- Force Tags E2E in Settings (m6 convention).
- Timeout set to 35 minutes for real LLM execution headroom.

Note: plan prompt (spec Step 4 — supervised-profile user intervention)
is not yet implemented as a CLI command. A documentation note in the
test indicates this should be added once the command is available.

ISSUES CLOSED: #758
 2026-03-30 13:46:21 +08:00
hurui200320 2651e15854 fix(cli): replace non-existent Container.resolve() with named provider calls 
The three plan.py call sites (tree, explain, correct) that originally used
container.resolve(DecisionService) were already fixed in master. This commit
completes the fix by aligning the Robot Framework test mocks to match the
corrected production code:

- robot/helper_m3_decision_validation_smoke.py: mock_container.resolve →
  mock_container.decision_service (plan correct dry-run path)
- robot/helper_m4_correction_subplan_smoke.py: container.resolve →
  container.decision_service (correction subplan path)

Both helpers previously passed silently because MagicMock auto-creates any
attribute, masking the mismatch between mock setup and actual code. Added
spec=Container to both mock containers so that accessing non-existent
attributes raises AttributeError, matching real container behavior.

Updated comment wording in features/container_resolve_crash.feature to
clarify that the bug is fixed and the scenarios serve as permanent
regression guards. The @tdd_expected_fail tag was already removed by the
TDD branch before this fix branch was created; the permanent @tdd_issue
and @tdd_issue_647 tags remain per TDD tag lifecycle rules.

Added CHANGELOG.md entry describing the fix.

ISSUES CLOSED: #647
 2026-03-30 04:45:56 +00:00
freemo 845cf61b47 test(e2e): workflow example 4 — multi-project dependency update (supervised profile) 
Implement WF04 E2E test coverage for multi-project dependency update
using the supervised automation profile. A parent plan targets four
projects (common-lib + 3 services), spawns child subplans per project,
executes in dependency order, validates per project, and applies in
dependency order.

Key components:
- Robot test (robot/e2e/wf04_multi_project.robot): full supervised
  workflow exercising plan use, strategize, execute, validate, apply,
  and final status. JSON output parsing via raw_decode for resilience.
  Test-level guard skips the entire test (visible SKIPPED in CI) when
  the LLM produces zero subplans, preventing silent AC-4/5/6/7 bypass.
  Count Decision Nodes keyword delegates to the Python helper to avoid
  duplicating recursive tree-walking logic.
- Snapshot helper (robot/e2e/wf04_snapshot_helper.py): collects
  deterministic parent/subplan metadata from the lifecycle service.
  Timestamps normalised to UTC. Depth-guarded count_decision_nodes()
  function (max_depth=50) for safe reuse from Robot keywords.
  sys.path.append (not insert) to avoid shadowing the standard library.
- Behave unit tests (features/wf04_snapshot_helper.feature + steps):
  17 scenarios covering _iso(), _enum_value(), count_decision_nodes(),
  and _build_snapshot() with mocked lifecycle service. Includes tests
  for no-subplans (verifies plan_id, project_scopes, validation_summary),
  with-subplans (verifies concrete mapped project names and subplan_count),
  child-plan-is-None (verifies empty-string defaults), unmapped resources,
  and exact-match aware-datetime UTC normalisation.

ISSUES CLOSED: #750
 2026-03-30 11:47:37 +08:00
Luis Mendes 7f078f75a5 fix(sandbox): make commit_all atomic per specification 
Changed SandboxManager.commit_all() from partial-commit semantics to
all-or-nothing atomic operation per specification requirement. On
partial failure, already-committed sandboxes are rolled back. Error
reporting indicates which sandbox failed and what was rolled back.
Added Behave scenarios verifying atomicity guarantee.

Hardened atomicity guarantees after code review:

- commit_all and _rollback_committed catch Exception (not just
  SandboxError) so unexpected errors cannot bypass rollback.
  Non-SandboxError exceptions are wrapped in a new AtomicCommitError
  (chaining the original as __cause__) that carries rolled_back_ids
  and failed_rollback_ids attributes so callers can programmatically
  determine rollback outcomes.

- _rollback_committed returns both rolled_back_ids and
  failed_rollback_ids; the error result metadata now carries
  both "rolled_back" and "rollback_failed" keys.

- _rollback_committed iterates in reverse (LIFO) order following
  the standard transaction-log undo pattern.  Clarified in
  docstring that this is distinct from the specification DAG-based
  "top-down" rollback ordering (line 24632).

- TransactionSandbox.rollback() from COMMITTED now raises
  SandboxRollbackError (database commits are irreversible)
  instead of silently transitioning to ROLLED_BACK.

- TransactionSandbox is now classified as non-rollbackable in
  commit_all alongside NoSandbox and committed last in the batch,
  since database COMMIT is irreversible.  Docstring corrected to
  match the raising behavior.

- CopyOnWriteSandbox and OverlaySandbox rollback-from-COMMITTED
  uses rename-based safe_restore() to prevent data loss when
  the copytree step fails after the original was removed.

- CopyOnWriteSandbox and OverlaySandbox rollback() now catches
  Exception (not just OSError), matching the broader catch used
  in _rollback_committed, so non-OSError exceptions from
  safe_restore set the status to ERRORED correctly.

- Extracted shared _fs_utils module (backup_directory,
  safe_restore, compute_diff) with symlink, permission, and
  timestamp preservation (including directory timestamps),
  replacing duplicated per-class _backup_directory and
  _compute_diff methods.

- backup_directory defers directory permissions and timestamps
  to a bottom-up post-walk pass, fixing incorrect mtime
  preservation (POSIX file creation inside a directory overwrites
  its mtime) and preventing restrictive source permissions from
  blocking backup writes.

- backup_directory skips non-regular files (FIFOs, sockets,
  device files) with a warning to prevent hangs on special files.

- CopyOnWriteSandbox and OverlaySandbox commit() now attempts
  to restore the original from the pre-commit backup when the
  file-copy phase fails midway, preventing partial corruption.
  If the restore itself fails the backup is preserved for manual
  recovery (cleanup() still removes it).

- CopyOnWriteSandbox and OverlaySandbox commit() error handler
  now catches Exception (not just OSError) so that unexpected
  errors during the file-copy phase also trigger pre-commit
  backup restoration, preventing partial corruption of the
  original directory.

- Pre-commit backup exception handler catches Exception (not
  just OSError) preventing temp directory leaks on non-OSError
  failures from backup_directory.

- Fixed _pre_commit_backup assignment timing: the backup
  reference is now assigned only AFTER backup_directory()
  succeeds, preventing safe_restore() from corrupting an intact
  original with a partial backup when backup_directory() fails
  (e.g. disk full).

- Rollback from COMMITTED with no pre-commit backup (no changes
  were applied) is now a no-op instead of raising
  SandboxRollbackError, preventing false rollback-failure
  reports in commit_all error metadata.

- commit_all logs a warning when NoSandbox or TransactionSandbox
  instances are present in the batch since their changes cannot
  be rolled back, which breaks the atomicity guarantee.

- Pre-commit backup is skipped when compute_diff returns no
  changes, avoiding a full directory copy for no-op commits.

- GitWorktreeSandbox clears _pre_merge_commit on commit failure
  so the stale value cannot be used by future code.

- commit_all docstring documents Raises clause for AtomicCommitError
  exception wrapping behavior.

- GitWorktreeSandbox.rollback() docstring warns about
  multi-worktree safety when rolling back from COMMITTED.

- Updated SandboxStatus transition diagram in protocol.py to
  clearly show the COMMITTED -> ROLLED_BACK path.

- Added spec-contradiction note (line 45938 vs 19193) in
  commit_all docstring.

- OverlaySandbox rollback from COMMITTED now properly remounts
  OverlayFS for real overlay (unmount, clean upper/work dirs,
  remount) and uses dirs_exist_ok=True for userspace fallback
  to prevent FileExistsError if rmtree silently fails.  The
  merged directory is reset from the restored original,
  preventing stale pre-rollback data from being exposed on
  re-activation via get_path() (which allows ROLLED_BACK status).

- OverlaySandbox rollback from COMMITTED now raises
  SandboxRollbackError if the OverlayFS unmount fails,
  preventing a double-mount attempt that would leave the
  sandbox in an inconsistent state.

- OverlaySandbox rollback from ACTIVE now uses dirs_exist_ok=True
  for userspace fallback to prevent FileExistsError when rmtree
  with ignore_errors=True silently fails.

- CopyOnWriteSandbox rollback from ACTIVE now uses
  dirs_exist_ok=True in copytree to prevent FileExistsError
  when rmtree with ignore_errors=True silently fails, matching
  the fix already applied to OverlaySandbox.

- Non-rollbackable sandboxes (NoSandbox, TransactionSandbox) are
  committed last in the batch so that all rollbackable sandboxes
  commit first; if any rollbackable sandbox fails, none of the
  non-rollbackable sandboxes will have committed yet.

- Moved NoSandbox and TransactionSandbox imports to module level
  in manager.py (no circular dependency exists).

- Pre-commit backups are now created on the same filesystem as
  the original directory (using dir= argument to mkdtemp),
  avoiding cross-device copy overhead and ensuring os.rename
  compatibility.

- safe_restore now renames the target into the mkdtemp directory
  instead of removing the mkdtemp dir first, eliminating the
  residual TOCTOU window between rmdir and rename.

- safe_restore catches BaseException (not just OSError) to
  ensure the original directory is always renamed back on
  unexpected errors, preventing the original from being left
  in the renamed-aside state.

- Added AtomicCommitError exception class to protocol.py
  carrying rolled_back_ids and failed_rollback_ids attributes.

- Exported AtomicCommitError from sandbox package __init__.py
  so callers can import it from the public API.

- Added BDD scenarios: LIFO rollback order, AtomicCommitError
  wrapping with RuntimeError cause and rollback metadata,
  _fs_utils backup/restore coverage, no-change commit rollback
  success, directory timestamp preservation, OverlaySandbox
  merged dir reset after COMMITTED rollback,
  CopyOnWriteSandbox rollback from COMMITTED restores original,
  GitWorktreeSandbox rollback from COMMITTED undoes merge,
  TransactionSandbox rollback from COMMITTED raises
  SandboxRollbackError about irreversible commit.

ISSUES CLOSED: #925

Post-review hardening (PR #1146 review findings):

- OverlaySandbox rollback from COMMITTED with no backup (no-op)
  now skips the merged directory reset entirely, preventing
  unnecessary unmount/remount or re-copy that could fail and
  turn a harmless no-op rollback into a SandboxRollbackError
  during commit_all atomic recovery.

- OverlaySandbox rollback no longer double-wraps
  SandboxRollbackError: the outer except Exception handler
  now has a preceding except SandboxRollbackError clause that
  re-raises directly, avoiding a confusing double-wrapped
  error chain.

- CopyOnWriteSandbox.get_path() now accepts ROLLED_BACK status
  for consistency with OverlaySandbox and the protocol status
  transition table (ROLLED_BACK -> ACTIVE).

- CopyOnWriteSandbox rollback from COMMITTED now resets the
  sandbox copy from the restored original via rmtree+copytree,
  preventing stale pre-rollback modifications from being
  exposed on re-activation.

- rollback_all now catches Exception (not just SandboxError)
  so that unexpected rollback errors do not prevent remaining
  sandboxes from being rolled back, consistent with the pattern
  already used in _rollback_committed.

- commit_all docstring now documents a thread-safety warning:
  the method is not safe for concurrent calls on the same
  plan_id since sandbox commit/rollback runs outside the lock.

- Fixed CHANGELOG.md whitespace inconsistencies (double leading
  spaces on two lines).

Post-review hardening round 2 (PR #1146 automated review):

- safe_restore now uses os.rename (O(1) atomic rename) instead
  of shutil.copytree (O(n) recursive copy) for the main restore
  path, since backup and target are always on the same
  filesystem.  This eliminates the ENOTEMPTY bug where a partial
  copytree failure left target_path partially populated, causing
  the recovery os.rename to fail and strand the original in the
  stale temp directory.

- OverlaySandbox.get_path() now transitions ROLLED_BACK to
  ACTIVE, matching CopyOnWriteSandbox and the protocol
  transition table (ROLLED_BACK -> ACTIVE).

- GitWorktreeSandbox.get_path() now accepts ROLLED_BACK status
  for consistency with all other sandbox implementations and
  the protocol transition table (ROLLED_BACK -> ACTIVE).

- rollback_all now also handles sandboxes in COMMITTED status
  (not just ACTIVE), consistent with the state machine allowing
  COMMITTED -> ROLLED_BACK.

- cleanup_all now catches Exception (not just SandboxError) so
  a single unexpected error does not abort cleanup of remaining
  sandboxes, consistent with _rollback_committed and
  rollback_all.

- Restructured CHANGELOG entry from a single ~90-line paragraph
  into structured sub-bullets for readability.

- Added BDD scenarios: no-op rollback from COMMITTED for
  CopyOnWriteSandbox and OverlaySandbox (zero-change commit),
  commit ordering verification (rollbackable before
  non-rollbackable).

Post-review hardening round 3 (PR #1146 deep automated review):

- cleanup_abandoned now catches Exception (not just SandboxError)
  so that unexpected errors (e.g. raw OSError, PermissionError)
  do not crash the loop and prevent remaining abandoned sandboxes
  from being cleaned up, consistent with cleanup_all,
  rollback_all, and _rollback_committed.

- OverlaySandbox._mount_overlay() now catches
  subprocess.TimeoutExpired (in addition to CalledProcessError
  and OSError), preventing create() from leaving the sandbox in
  PENDING status when mount hangs beyond the timeout.

- OverlaySandbox._unmount_overlay() now catches
  subprocess.TimeoutExpired (in addition to CalledProcessError
  and OSError), preventing cleanup() from leaving the sandbox
  in a zombie state when umount hangs beyond the timeout.

- OverlaySandbox._mount_overlay() validates that overlay paths
  do not contain commas, which would corrupt the OverlayFS mount
  options string (comma is the mount option delimiter).

- GitWorktreeSandbox.commit() now checks git diff return code
  so that a failed diff command raises CalledProcessError instead
  of silently concluding there are no changes and skipping the
  merge.

- safe_restore cleanup of the temporary rollback container now
  runs in a finally block, preventing a temp directory leak
  when the rename fails and the exception is re-raised.

- Fixed misleading BDD step name: "backup path that will cause
  copytree to fail" renamed to "backup path that will cause
  rename to fail" since safe_restore now uses os.rename.
 2026-03-29 18:57:15 +01:00
Luis Mendes f678d611bb feat(db): add correction_attempts table per specification DDL 
Added CorrectionAttemptModel SQLAlchemy model with all spec-defined columns
(correction_attempt_id, plan_id, original_decision_id, new_decision_id,
mode, guidance, archived_artifacts_path, state, created_at, completed_at).
Added FK constraints to v3_plans and decisions tables. Created Alembic
migration and idx_corrections_plan index. Added repository layer for
CRUD operations.

Addressed code review feedback (rounds 1-12):
- Replaced Any type annotations with typed CorrectionAttemptRecord
  signatures using TYPE_CHECKING imports.
- Replaced fragile time.sleep with deterministic timestamps.
- Added cascade deletion test with PRAGMA foreign_keys=ON.
- Changed update_state() to accept typed enum and datetime params.
- Added guidance non-empty validator with max_length=10_000.
- Added spec-aligned server_default for created_at column.
- Fixed timezone handling in to_domain() and from_domain().
- Added spec-defined lifecycle state transition validation via
  validate_correction_state_transition() domain function.
- Improved FK-violation error messages in create() and update_state().
- Normalised timestamp to millisecond precision matching SQLite
  server_default strftime('%f') output.
- Added auto-set completed_at on terminal transitions.
- Added CorrectionAttemptRecord field validators (strip, non-empty).
- Changed original_decision_id FK from CASCADE to RESTRICT matching
  spec DDL default, preserving correction audit trail.
- Added input validation in update_state() for new_decision_id and
  archived_artifacts_path (empty/whitespace rejection).
- Fixed dirty-session bug by moving validation before ORM mutations.
- Extracted _SQLITE_TIMESTAMP_MS_LEN constant for timestamp truncation.

Addressed thirteenth code review feedback:
- Changed InvalidCorrectionStateTransitionError base class from
  DatabaseError to BusinessRuleViolation per CONTRIBUTING.md exception
  semantics (state transition is a business rule, not a database error;
  prevents incorrect retries by @database_retry decorator).
- Changed new_decision_id FK from SET NULL to RESTRICT matching the
  spec DDL default (no ON DELETE clause) and consistent with the
  RESTRICT approach used for original_decision_id.
- Changed update_state() input validation for new_decision_id and
  archived_artifacts_path from DatabaseError to ValueError per
  CONTRIBUTING.md argument validation guidelines.
- Defensive to_domain() coercion now defaults corrupted state to
  'failed' (terminal) instead of 'pending', preventing re-execution
  of completed/failed corrections with corrupted DB values.
- Extracted format_sqlite_timestamp() helper and SQLITE_TIMESTAMP_MS_LEN
  public constant, removing duplicated timestamp formatting logic
  between from_domain() and update_state().
- Added code comment explaining CASCADE on plan_id FK as a codebase
  convention deviation from spec DDL default.
- Added BDD scenario verifying RESTRICT FK on original_decision_id
  blocks decision deletion.
- Replaced weak cross-plan isolation test with stronger two-plan
  scenario verifying list_by_plan returns only each plan's attempts.
- Fixed hardcoded assertion in step_check_archived_path to use
  context variable.
- 45 BDD scenarios and 5 Robot integration tests.

Addressed fourteenth code review feedback:
- Added ORM-level relationship(cascade="all, delete-orphan") on
  LifecyclePlanModel for CorrectionAttemptModel, consistent with all
  other v3_plans child tables, ensuring ORM-level cascade deletes
  work even when SQLite FK enforcement is disabled.
- Added defensive to_domain() coercion for corrupted guidance column
  (defaults to "[corrupted]" with warning log), consistent with
  existing mode/state coercion pattern.
- Added ValueError guard in format_sqlite_timestamp() rejecting naive
  datetimes per CONTRIBUTING.md fail-fast argument validation.
- Fixed stale spec DDL line reference in CorrectionAttemptModel
  docstring.
- Fixed duplicated docstring on SQLITE_TIMESTAMP_MS_LEN constant.

Addressed fifteenth code review feedback:
- Fixed update_state() to defensively handle corrupted DB state values
  via try/except ValueError coercion to FAILED terminal state with
  warning log, consistent with to_domain() defensive coercion pattern.
- Strengthened RESTRICT FK BDD assertion to verify exception type
  (IntegrityError/DatabaseError) instead of only checking presence.
- Split multi-When/Then cross-plan isolation BDD scenario into
  idiomatic single-When/Then scenarios per Gherkin best practice.
- 53 BDD scenarios (was 45) and 5 Robot integration tests.

ISSUES CLOSED: #920
 2026-03-29 16:23:04 +01:00
brent.edwards 3abf25f17f test: add TDD bug-capture test for #1038 — validation add --required flag (#1133) 
## Summary

Adds TDD bug-capture tests for bug #1038 — the `agents validation add` command is missing `--required`/`--informational` flags that the specification describes at line 22334 and in numerous walkthrough examples.

### Behave BDD Tests

Four Behave BDD scenarios verify the correct expected behavior:

1. **`--required` flag is accepted** — invokes `validation add --config <file> --required` and asserts exit code 0 and mode `"required"` in output.
2. **`--informational` flag is accepted** — same pattern with `--informational`.
3. **`--required` overrides YAML config mode** — creates a YAML config with `mode: informational`, invokes with `--required`, and asserts the CLI output and service layer both reflect `mode: "required"`.
4. **`--informational` overrides YAML config mode** — creates a YAML config with `mode: required`, invokes with `--informational`, and asserts the CLI output and service layer both reflect `mode: "informational"`.

### Robot Framework Integration Tests

Four Robot Framework integration tests exercise the same CLI paths as the Behave scenarios via a subprocess helper script:

1. **TDD Validation Add Required Flag Accepted** — verifies `--required` is recognised by the CLI.
2. **TDD Validation Add Informational Flag Accepted** — verifies `--informational` is recognised.
3. **TDD Validation Add Required Flag Overrides YAML Config** — verifies `--required` overrides a YAML config with `mode: informational`.
4. **TDD Validation Add Informational Flag Overrides YAML Config** — verifies `--informational` overrides a YAML config with `mode: required`.

The helper script (`robot/helper_tdd_validation_required_flag.py`) uses `typer.testing.CliRunner` with mocked services, following the same pattern as other TDD Robot helpers (e.g., `helper_tdd_plan_apply_yes_flag.py`).

### Tags

All scenarios (Behave and Robot) are tagged `@tdd_bug @tdd_bug_1038 @tdd_expected_fail`. Because the bug is still present, the underlying assertions fail (proving the bug exists) and the `@tdd_expected_fail` tag inverts the result so CI passes. Once bug #1038 is fixed, the tag must be removed.

### Spec Contradiction Note

Rui Hu's investigation (issue #1038 comment #70755) found that the formal CLI reference (specification.md lines 9279–9290) does NOT include `--required`/`--informational` flags — they appear only in walkthrough examples and specification.md line 22334. Additionally, specification.md line 30761 states: "For entity registration commands (`actor add`, `skill add`, `tool add`, `validation add`, …), the YAML configuration file is the sole source of truth — the `--config` file fully defines the entity and no CLI override flags are accepted." The resolution may be to add the flags to the CLI OR to clean up the spec. This is documented in both the feature file header and step definition module docstring.

### Key Design Decisions

- **YAML configs use flat `mode` key** — matches what `Validation.from_config()` actually reads (`config.get("mode", "required")`). The specification's Validation Configuration schema (specification.md line 34109) nests mode under a `validation` key, but the current `from_config()` reads a flat top-level `mode` key. A comment in the step definitions documents this discrepancy.
- **Mock uses `side_effect = lambda v: v`** — the `register_tool` mock returns the actual Validation object passed by the CLI, so the CLI output reflects real CLI processing rather than hard-coded mock configuration. This prevents false positives where the mock masks an incomplete fix.
- **False-positive guard** — the Then step verifying mode checks both CLI output (via `f"mode: {expected_mode}"` prefix match) AND `register_tool` call args to prevent the mock from masking an incomplete fix.
- **Robot helper follows established pattern** — uses `typer.testing.CliRunner` with mocked `_get_tool_registry_service`, matching the pattern from `helper_tdd_plan_apply_yes_flag.py`. Reports real outcome (exit 0 + sentinel on bug fixed, exit 1 on bug present) and relies on `tdd_expected_fail_listener` for result inversion.
- **Symmetric override scenarios** — both directions are tested: `--required` overriding informational YAML config, and `--informational` overriding required YAML config.
- **Positional NAME omitted** — the bug report includes a positional NAME argument, but that is a separate spec inconsistency not under test here.
- **Mutual exclusivity deferred** — testing what happens when both `--required` and `--informational` are passed simultaneously is deferred to the bug-fix PR for #1038.

Closes #1102

Reviewed-on: cleveragents/cleveragents-core#1133
Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>
Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
 2026-03-28 18:09:27 +00:00
brent.edwards bc6a41deb6 tdd(cli): prevent actor list from triggering database updates (#1151) 
## Summary

- Fix bug #797: `agents actor list` no longer triggers database writes (`upsert_actor`, `set_default_actor`) by removing `ensure_built_in_actors()` from `ActorRegistry.list()` and `ActorRegistry.list_actors()`
- Include TDD regression tests from #841 with `@tdd_expected_fail` removed per Bug Fix Workflow
- Update three existing test suites that relied on the old behavior to call `ensure_built_in_actors()` explicitly

## Root Cause

`ActorRegistry.list_actors()` and `ActorRegistry.list()` both unconditionally called `self.ensure_built_in_actors()` before delegating to the actor service. `ensure_built_in_actors()` iterates all configured providers and calls `_actor_service.upsert_actor()` for each — a database WRITE operation. It may also call `_actor_service.set_default_actor()` if no default exists — another WRITE. This means every read-only `agents actor list` command triggered database writes and could prompt for pending migrations on fresh checkouts.

## Changes

### Bug Fix
- **`src/cleveragents/actor/registry.py`** — Removed `self.ensure_built_in_actors()` from `list()` and `list_actors()`. Both methods now delegate directly to the service layer without triggering writes. All write-heavy methods (`add`, `upsert_actor`, `get`, `get_actor`, `remove`, `remove_actor`, `set_default_actor`, `get_default_actor`) still call `ensure_built_in_actors()`.

### TDD Tests (from #841, `@tdd_expected_fail` removed)
- **`features/tdd_actor_list_no_db_update.feature`** — 2 Behave scenarios verifying `upsert_actor` and `set_default_actor` are not called during `actor list`
- **`features/steps/tdd_actor_list_no_db_update_steps.py`** — Step definitions
- **`robot/tdd_actor_list_no_db_update.robot`** — 2 Robot Framework integration tests
- **`robot/helper_tdd_actor_list_no_db_update.py`** — Robot helper script

### Test Adjustments
Three existing test suites relied on the old (buggy) behavior where `list_actors()` called `ensure_built_in_actors()`:
1. **`features/consolidated_actor.feature`** — Scenario updated to explicitly call `ensure_built_in_actors()` before `list_actors()`
2. **`features/steps/tdd_actor_list_validation_steps.py`** + **`robot/helper_tdd_actor_list_validation.py`** (bug #592) — Updated to call `ensure_built_in_actors()` explicitly before CLI invocation
3. **`features/steps/actor_list_empty_steps.py`** (bug #592) — Updated with explicit `ensure_built_in_actors()` call and capturing upsert pattern

## Quality Gates

| Gate | Result |
|------|--------|
| `nox -e lint` | ✅ Pass |
| `nox -e typecheck` | ✅ Pass (0 errors) |
| `nox -e unit_tests` | ✅ Pass (468 features, 12367 scenarios, 0 failures) |
| `nox -e integration_tests` | ⚠️ 6 pre-existing failures (timeouts/OOM) |
| `nox -e coverage_report` | ✅ 98% (>= 97% threshold) |

The 6 integration test failures are pre-existing infrastructure issues (SIGTERM/SIGKILL timeouts) unrelated to this change: Container Resolve Crash (3), M3 E2E Verification (2), Resource CLI (1).

Closes #797

Reviewed-on: cleveragents/cleveragents-core#1151
Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>
Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
 2026-03-28 05:27:14 +00:00
brent.edwards b59100cc6c test: add TDD bug-capture test for #1141 — session create persistence (#1144) 
## Summary

Adds TDD bug-capture coverage for bug #1141 (`session create` -> `session list` lifecycle) on metadata branch `tdd/m3-session-create-persist`.

### What changed

- Added Behave bug-capture feature: `features/tdd_session_create_persist.feature`
  - Required tags: `@tdd_bug @tdd_bug_1141 @tdd_expected_fail`
  - Scenario path: `init --force --yes` -> `session list --format json` -> `session create` -> `session list --format json`
- Added Behave step definitions: `features/steps/tdd_session_create_persist_steps.py`
  - Uses root CLI app (`cleveragents.cli.main:app`) for realistic command routing
  - Asserts expected list totals; underlying assertion intentionally fails while bug is present
- Added Robot E2E bug-capture test: `robot/e2e/e2e_session_create_persist.robot`
  - Required tags: `E2E`, `tdd_expected_fail`, `tdd_bug`, `tdd_bug_1141`
  - Includes explicit in-file note to remove `tdd_expected_fail` when #1141 is fixed
- Updated changelog (`CHANGELOG.md`, Unreleased)

### Notes

- The issue subtask referenced `robot/e2e/e2e_session_lifecycle.robot`, which is not present on current `master`; equivalent E2E coverage is implemented in `robot/e2e/e2e_session_create_persist.robot`.
- The branch was force-updated to remove stale merge-based history and keep an atomic, rebase-clean commit for this issue.

## Quality gates

| Gate | Result |
|---|---|
| `nox -s lint` | ✅ pass |
| `nox -s typecheck` | ✅ pass |
| `nox -s unit_tests -- features/tdd_session_create_persist.feature` | ✅ pass (TDD inversion active; underlying assert fails) |
| `nox -s integration_tests -- robot/e2e/e2e_session_create_persist.robot` | ✅ pass |
| `nox -s e2e_tests` | ✅ pass |
| `nox -s coverage_report` | ✅ pass (97.66173849218832%) |
| `nox` (full default suite) | ✅ pass |

## Related issue

Closes #1142

Reviewed-on: cleveragents/cleveragents-core#1144
Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>
Co-authored-by: Brent Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent Edwards <brent.edwards@cleverthis.com>
 2026-03-28 04:34:47 +00:00
brent.edwards 26632f79e9 test: add TDD bug-capture test for #1079 — project context set missing flag (#1130) 
## Summary

This PR adds TDD bug-capture tests for bug #1079 (`project context set` missing `--execution-env-priority` flag). Per the Bug Fix Workflow in CONTRIBUTING.md, the first step in fixing any bug is to write a test that proves the bug exists.

### What was done

- **Behave unit tests** (6 scenarios in `features/project_context_set_exec_env_priority.feature`):
  - `project context set --execution-env-priority override` with `--execution-environment` should succeed and persist
  - `project context set --execution-env-priority fallback` with `--execution-environment` should succeed and persist
  - `--execution-env-priority` without `--execution-environment` should be rejected
  - Default to `fallback` when only `--execution-environment` is specified
  - Invalid `--execution-env-priority` value should be rejected
  - `project context show` should reflect persisted `execution_env_priority` in JSON output

- **Robot integration tests** (3 test cases in `robot/project_context_set_exec_env_priority.robot`):
  - Override acceptance with persistence verification
  - Fallback acceptance with persistence verification
  - Full round-trip persistence check

All tests are tagged `@tdd_bug @tdd_bug_1079 @tdd_expected_fail`. The underlying assertions fail (confirming the bug exists — `--execution-env-priority` is "No such option"), and the `@tdd_expected_fail` tag inverts the result so CI passes.

### Bug confirmed

The `context_set()` function in `cleveragents.cli.commands.project_context` does not accept `execution_env_priority` as a parameter. The specification (§Execution Environment Routing, precedence table) requires this flag at precedence level 2 for project-level execution environment priority control.

### Quality gates

| Gate | Result |
|------|--------|
| `nox -s lint` | PASS |
| `nox -s typecheck` | PASS (0 errors) |
| `nox -s unit_tests` | PASS (12236 scenarios, 0 failed) |
| `nox -s integration_tests` | My 3 tests pass (13 pre-existing failures) |
| `nox -s coverage_report` | PASS (98%, threshold 97%) |

Closes #1100

Reviewed-on: cleveragents/cleveragents-core#1130
Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>
Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
 2026-03-28 02:14:55 +00:00
brent.edwards b46a0e6102 feat(plan): implement real revert-mode re-execution from decision point (#1153) 
## Summary

Implements the full revert-mode re-execution pipeline from the specification (§ Correction Flow, Revert Mode), enabling `agents plan correct --mode=revert` to truly re-execute from the targeted decision point rather than only logically marking decisions as superseded.

### Changes

- **Resource rollback**: `CorrectionService.execute_revert` now delegates to `CheckpointService.rollback_to_checkpoint` when a decision-aligned checkpoint exists, performing real `git reset --hard` in the sandbox. Gracefully degrades when checkpointing is unavailable.
- **Reasoning rollback**: Extracts `actor_state_ref` from the target decision's `ContextSnapshot` for downstream LangGraph actor restoration.
- **Guidance injection**: Generates a `user_intervention` decision ID for callers to record correction guidance in the decision tree.
- **Phase transition**: Signals that the plan should re-enter Strategize via `phase_transition_target="strategize"` on the result.
- **Model extension**: Added `checkpoint_restored`, `actor_state_ref`, `user_intervention_decision_id`, and `phase_transition_target` fields to `CorrectionResult` with backward-compatible defaults.
- **Dispatch update**: `execute_correction` now accepts and forwards a `decisions` parameter.

### Design Approach

Uses a signal-based architecture: rather than having `CorrectionService` directly manipulate plan state or LangGraph actors, the enhanced `execute_revert` returns signal fields on `CorrectionResult` that downstream consumers use for the full pipeline. This preserves separation of concerns.

### Tests

- **Behave**: 16 new BDD scenarios in `features/revert_re_execution.feature`
- **Robot**: 7 integration tests in `robot/revert_re_execution.robot`
- **Coverage**: 98% (above 97% threshold)

### Quality Gates

| Gate | Status |
|------|--------|
| `nox -s lint` | Pass |
| `nox -s typecheck` | Pass (0 errors) |
| `nox -s unit_tests` | Pass (12376 scenarios, 0 failed) |
| `nox -s integration_tests` | Pass (1631 passed, 3 pre-existing failures #647) |
| `nox -s e2e_tests` | Pass (37 passed) |
| `nox -s coverage_report` | Pass (98%) |

Closes #844

Reviewed-on: cleveragents/cleveragents-core#1153
Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
 2026-03-28 00:57:51 +00:00