Add Behave feature/step pairs that exercise uncovered branches across handlers, LSP, CLI, and service layers to reach the coverage gate.
ISSUES CLOSED: #1232
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
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
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
## Summary
Adds the missing `--execution-env-priority` flag to the `agents plan use` command, aligning the CLI with the specification (spec line 12501). The flag accepts `fallback` (default) or `override` and controls execution environment routing precedence per ADR-043:
- **`override`**: The specified execution environment always wins, bypassing devcontainer auto-detection.
- **`fallback`**: The specified environment defers to auto-detected devcontainers or project-level overrides.
### Changes
- **Domain model** (`cleveragents.domain.models.core.plan`):
- Added `ExecutionEnvPriority` StrEnum with `FALLBACK`/`OVERRIDE` values.
- Changed `execution_env_priority` field type to `ExecutionEnvPriority | None` (leverages Pydantic enum validation).
- Added `@model_validator` enforcing that `execution_env_priority` requires `execution_environment` (domain-level fail-fast invariant). Verified `validate_assignment=True` is set on `Plan.model_config`.
- Updated `Plan.as_cli_dict()` to include `execution_environment` and `execution_env_priority`, with fallback default of `"fallback"` for pre-migration data where `execution_env_priority` is `None`.
- **CLI** (`cleveragents.cli.commands.plan`):
- Added `--execution-env-priority` parameter to `use_action`.
- Validation: priority requires `--execution-environment`, enum value validation, case-insensitive input.
- Defaults to `"fallback"` when `--execution-environment` is set without explicit priority.
- Updated `_print_lifecycle_plan` and `_plan_spec_dict` to display the priority, defaulting to `"fallback"` for pre-migration data.
- Updated `_plan_spec_dict` docstring to mention new keys.
- Hoisted `ExecutionEnvPriority` import to function-entry deferred imports in both `_plan_spec_dict` and `_print_lifecycle_plan` (no longer conditional on execution environment being set).
- Guarded `service.save_plan(plan)` with `has_overrides` flag so it is only called when CLI overrides were actually applied.
- **Persistence** (`cleveragents.infrastructure.database`):
- Added `execution_environment` (`String(255)`, nullable) and `execution_env_priority` (`String(20)`, nullable) columns to `LifecyclePlanModel`.
- Updated `from_domain()`/`to_domain()` for round-trip serialization including `ExecutionEnvPriority` enum reconstruction. Uses direct attribute access (`plan.execution_environment`) instead of defensive `getattr` — Plan fields always exist on the Pydantic BaseModel.
- Updated `LifecyclePlanRepository.update()` to persist both fields using direct attribute access.
- Added Alembic migration `m4_003_plan_env_columns` adding both columns to the `v3_plans` table. Uses `String(255)` for `execution_environment` to accommodate namespaced resource names. Descends from `m6_005_profile_guards_json`.
- **Service** (`cleveragents.application.services.plan_lifecycle_service`):
- Added `save_plan()` public convenience method for callers that need to re-persist after post-creation mutations.
- **Tests**:
- 18 Behave scenarios covering:
- CLI acceptance criteria (valid values, defaults, validation errors, output display, case-insensitive input, service invocation with `call_args` verification).
- Domain model validator invariant: construction with priority but no environment raises `ValueError`; construction with both fields succeeds.
- `ExecutionEnvPriority` enum: values verification, `StrEnum` subclass assertion.
- `Plan.as_cli_dict()`: includes both fields when set, omits both when `None`, defaults priority to `"fallback"` for pre-migration data.
- DB round-trip serialization: `from_domain()` → `to_domain()` preserves both fields; preserves `None` values.
- 5 Robot Framework integration tests.
- Updated pre-existing `SimpleNamespace`-based plan test fixtures in `database_models_lifecycle_coverage_steps`, `database_models_new_coverage_steps`, `database_models_coverage_r2_steps`, and `repositories_error_handling_coverage_steps` to include `execution_environment` and `execution_env_priority` attributes.
- Simplified Robot helper `sys.path` pattern to standard approach.
- **Changelog**: Updated per CONTRIBUTING.md requirements.
### Review Fixes (Brent Edwards, Review #2384)
- **P2 #1 — Defensive `getattr`**: Replaced `getattr(plan, "execution_environment", None)` with direct `plan.execution_environment` access in both `LifecyclePlanModel.from_domain()` and `LifecyclePlanRepository.update()`. Plan is a Pydantic BaseModel with `default=None`, so the field always exists. Updated 4 pre-existing `SimpleNamespace`-based test fixtures to include the new attributes.
- **P2 #2 — Late conditional import**: Hoisted `ExecutionEnvPriority` import from inside conditional blocks to function-entry deferred imports in both `_plan_spec_dict` and `_print_lifecycle_plan`.
- **P3 — Migration naming**: Acknowledged as deferred (cosmetic only). Updated `m4_003` to descend from `m6_005_profile_guards_json` (post-rebase chain fix).
- **Rebase**: Branch rebased onto latest `master` with merge conflict in `CHANGELOG.md` resolved.
### Deferred Items
- **Partial failure atomicity** (#8 from review): If `save_plan()` fails after `use_action()` succeeds, the plan exists in the DB without CLI overrides. This requires service-layer restructuring beyond the scope of this ticket.
- **Alembic migration naming** (#11 from review): Migration `m4_003` depends on `m6_005`, creating non-sequential naming. Renaming an existing migration risks breaking the chain for anyone who has already applied it.
### Quality Gates
| Session | Result |
|---------|--------|
| lint | PASS |
| typecheck | PASS (0 errors) |
| unit_tests | PASS (11,125 scenarios, 0 failures) |
| integration_tests | PASS (1,562 tests, 0 failures) |
| coverage_report | 97% (threshold: 97%) |
Closes#886
Reviewed-on: cleveragents/cleveragents-core#972
Reviewed-by: Brent Edwards <brent.edwards@cleverthis.com>
Co-authored-by: Rui Hu <rui.hu@cleverthis.com>
Co-committed-by: Rui Hu <rui.hu@cleverthis.com>
The automation-profile CLI commands used an _InMemoryProfileRepository
(a Python dict) that lost all data between CLI process invocations.
Profiles created with "automation-profile add" were invisible to
subsequent "automation-profile show" or "list" calls because each CLI
command is a separate process with a fresh empty dict.
Changes:
- Replaced _InMemoryProfileRepository with the real
AutomationProfileRepository from the infrastructure layer, wired
via the DI container following the same pattern as tool.py and
session.py.
- Added auto_commit support to AutomationProfileRepository (matching
the existing SessionRepository pattern) so that CLI commands running
outside a UnitOfWork commit each operation automatically.
- Added safety_json and guards_json Text columns to the
automation_profiles table (Alembic migration m6_005) for full-fidelity
round-trip of the AutomationGuard and SafetyProfile sub-models.
Previously, guards and several safety fields (max_cost_per_plan,
max_retries_per_step, etc.) were silently dropped on persistence.
- Updated _from_domain, _to_domain, and _update_row to serialize and
deserialize the full guard and safety sub-models via JSON, with
backward-compatible fallback to legacy scalar columns.
Refs: #746
Implement ContainerToolExecutor for delegating tool invocations to
devcontainer environments with full I/O forwarding. Add PathMapper for
bidirectional host/container path translation. Wire container routing
into ToolRunner with graceful fallback when no executor is configured.
Add container_metadata field to ToolInvocation for tracking execution
context.
New modules:
- tool/container_executor.py: ContainerToolExecutor, ContainerConfig,
ContainerMetadata, ContainerExecutionError, ContainerTimeoutError
- tool/path_mapper.py: PathMapper with host_to_container/container_to_host
Modified:
- tool/runner.py: container execution routing via ExecutionEnvironment
- domain/models/core/change.py: container_metadata on ToolInvocation
- tool/__init__.py: new public exports
Review fixes applied:
- Add Alembic migration m6_004 for container_metadata_json column
- Enforce _MAX_OUTPUT_BYTES (50 MiB) truncation in _run_command()
- Fix path traversal in sync_results_to_host (Path.is_relative_to)
- Allow spaces in _looks_like_path() for valid filesystem paths
- Preserve negative exit codes from signal kills in metadata
- Add default=str to json.dumps(invocation.arguments) safety net
- Log warnings when path mapping recursion depth exceeded
- Warn when devcontainer binary not found on PATH
- Use default allow_nan for host-path JSON validation in runner.py
(only container path requires RFC 7159 strict mode)
- Reject URL-like patterns in _looks_like_path() to avoid false
positives on API routes, protocol-relative URIs, and query strings
- Add extract_container_metadata() static helper on
ContainerToolExecutor as bridge for ToolInvocation wiring
- Use raw_stdout bytes in sync_results_to_host to prevent
binary file corruption from text-mode decode/re-encode
- Apply posixpath.normpath() in workspace_folder validator and
reject path components containing '..'
- Check result.timed_out in sync_results_to_host and raise
ContainerTimeoutError instead of always raising ContainerExecutionError
- Detect overlapping host_root/container_root in PathMapper and
raise ValueError to prevent corrupt bidirectional mappings
- Wrap host-side I/O in sync_results_to_host with try/except
OSError to produce ContainerExecutionError on write failure
- Enforce int(timeout) in _build_exec_command to prevent shell
injection via malicious objects with __str__ methods
- Change ToolResult validator from 'not self.error' to
'self.error is None' so empty-string errors are accepted
- Iterate required list directly in ToolRunner schema validation
to detect fields listed in required but absent from properties
Closes#515
m6_004_resource_type_inherits and m7_001_repo_indexing_tables both had
down_revision='m6_003_async_jobs_table', creating two Alembic heads.
Chain m6_004 after m7_001 so the migration graph is linear.
Add RepoIndexingService with incremental refresh, language detection,
SHA-256 hashing, and policy enforcement. Includes domain models,
DB persistence, DI wiring, Behave/Robot/ASV tests, and reference docs.
ISSUES CLOSED: #195
- Add AsyncJob domain model with status state machine and Pydantic validation
- Add AsyncWorker service with configurable concurrency and job store
- Add CancellationToken, WorkerHealthReport, InMemoryJobStore
- Add AsyncJobModel SQLAlchemy model and Alembic migration (m6_003)
- Add 5 async config keys to Settings (worker_id, concurrency, poll_interval, max_retries, timeout)
- Add _check_async_worker_health diagnostic check in system.py
- Add comprehensive Behave BDD tests (~60 scenarios) with full step definitions
- Add Robot Framework integration tests (6 smoke tests)
- Add ASV benchmark suite for async execution
- Add architecture documentation
- Update vulture_whitelist with new public API symbols
- All quality gates pass: lint, typecheck, unit_tests, integration_tests, coverage_report (97%)
1. Wire async job creation into PlanLifecycleService:
- Add optional job_store parameter to __init__
- Add _maybe_enqueue_async_job() helper that checks settings.async_enabled
and job store presence before creating and enqueuing an AsyncJob
- Call helper from execute_plan() (phase="execute") and apply_plan()
(phase="apply") after phase transitions
- When async is disabled or no job store is configured, behaviour is
unchanged (silent no-op)
2. Redact secrets in failed job error messages:
- Apply shared.redaction.redact_value() to the error string before
persisting to AsyncJob.error_message, preventing accidental secret
leakage (e.g. API keys in exception text) into the audit trail
Documentation:
- S1: Added specification reconciliation note (ADR-style) to
async_architecture.md addressing tension between "No Plan Queuing"
clause and the async subsystem authorised by issue #312
ISSUES CLOSED: #312
Add SafetyProfile domain model with configurable safety constraints
(checkpoint, human approval, cost limits, retry limits, skill
categories, sandbox requirement). Integrate into Action model with
from_config/as_cli_dict support, and persist via LifecycleActionModel
safety_profile_json column.
Include resolve_safety_profile() stub that raises NotImplementedError
for local mode, signalling that real enforcement is deferred.
Add comprehensive test coverage:
- 20 BDD scenarios in features/safety_profile.feature
- 6 Robot Framework smoke tests
- 5 ASV benchmark suites (11 timing functions)
Add docs/reference/safety_profile.md reference documentation.
ISSUES CLOSED: #332
Add Alembic migration m4_002_skill_flattened_tools to extend the skills
table with five new columns: flattened_tools_json, includes_json,
capability_summary_json, yaml_text, and flattening_hash (SHA-256). A
defence-in-depth uniqueness constraint (uq_skills_name) is also added.
Update SkillModel with the new column definitions and extend
SkillRepository with update_flattened_tools(), get_flattened_tools(),
needs_refresh(), recompute_flattening_hash(), and
invalidate_cached_summaries() methods. The existing update() method
now nulls all cached fields on mutation (hash-based invalidation).
All new repository methods follow the session-factory pattern with
@database_retry and flush-but-don-t-commit semantics. Structured
logging via structlog records cache updates and invalidations.
Database schema docs updated with the new skills table columns and a
persistence-field-to-domain-model mapping table.
Tests:
- 6 Behave scenarios covering create, invalidation, hash staleness,
refresh recomputation, uniqueness constraint, and namespace filtering
- 2 Robot Framework smoke tests (round-trip and invalidation)
- 3 ASV benchmarks (persist, refresh check, namespace list)
ISSUES CLOSED: #166
After rebasing onto master, m4_001_decision_tables and
d0_002_merge_changeset_and_locks were both leaf heads. Update
down_revision so the decision migration chains after the merge
migration, restoring a single Alembic head.
ISSUES CLOSED: #171
Add decision persistence layer with DecisionRepository, DecisionModel,
and Alembic migration. Includes tree queries (BFS traversal via deque,
path-to-root), superseded lookup, ordered decision path retrieval,
concrete Decision type annotations (via TYPE_CHECKING), and comprehensive
test coverage (Behave BDD, Robot Framework, ASV benchmarks). Updated
database_schema.md, CHANGELOG.md, and CONTRIBUTORS.md.
ISSUES CLOSED: #171
Implemented plan-level and project-level locking with configurable timeouts.
Added locks table via Alembic migration storing owner_id, resource_type,
resource_id, acquired_at, and expires_at. Locks enforced in
PlanLifecycleService transitions. Support for re-entrant acquisition,
lock renewal, graceful shutdown release, and startup cleanup of expired
locks. Added diagnostics check for stale lock reporting.
ISSUES CLOSED: #327
freemo
CoreRasurae
hurui200320
hamza.khyari
khyari hamza
brent.edwards
Jeffrey Phillips Freeman