forked from HAL9000/cleveragents-core
fix/ci-optimize-e2e-tests-push
104 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
9e93ea5fc6 |
fix(persistence): close session in AutomationProfileRepository auto_commit finally block
Add missing `finally: if self._auto_commit: session.close()` blocks to all four public session-creating methods in `AutomationProfileRepository`: `get_by_name()`, `list_all()`, `upsert()`, and `delete()`. Without these blocks, sessions were never closed when the repository operated in auto_commit mode, causing a slow session leak that could exhaust the connection pool over time. The fix mirrors the pattern already used by `SessionRepository`, which correctly closes its session in a `finally` block for every public method. Removed the `@tdd_expected_fail` tag from the TDD test (`tdd_automation_profile_session_leak.feature`) so the six scenarios now run as normal regression tests (four original plus two new scenarios for `get_by_name` and `list_all`). ISSUES CLOSED: #987 |
||
|
|
36e6a4a520 |
fix(cli): add Name column, Summary panel fields, and OK message to session list rich output (#1570)
Co-authored-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me> Co-committed-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me> |
||
|
|
09be821a38 |
fix(cli): make version command show actual git commit (#1520)
Implements issue #1520. The version command now displays the actual git commit SHA to help identify which version of the code is running. Co-authored-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me> Co-committed-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me> |
||
|
|
bcf54d08c9 | Merge pull request 'feat(plugin): register spec-defined extension points in PluginManager' (#1217) from feature/plugin-extension-points into master | ||
|
|
0989f58dc8 |
feat(estimation): wire actor.default.estimation config fallback and Strategize-to-Estimate lifecycle hook (#1310)
Co-authored-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me> Co-committed-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me> |
||
|
|
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> |
||
|
|
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> |
||
|
|
3f5ca4e869 |
feat(plugin): register spec-defined extension points in PluginManager
Registered all 30 spec-defined extension points in the ExtensionPointRegistry with typed Protocol interfaces for each category: context strategies (12), output renderers (3), validation runners (2), tool providers (2), skill providers (2), resource handlers (2), A2A transports (2), event handlers (2), config sources (2), and safety guardrails (1). Added extension_protocols.py with Protocol definitions and extension_catalog.py with the registration catalog. Wired context subsystem to query context.* extension points at initialization. ISSUES CLOSED: #939 |
||
|
|
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
|
||
|
|
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 |
||
|
|
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 |
||
|
|
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. |
||
|
|
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
|
||
|
|
7cf03f03f4 |
fix(action): query persisted actions from database in list_actions
list_actions() previously merged in-memory cache with the database but used the cache as the primary source. Actions created by previous CLI invocations were only stored in the database and were invisible to the current process. When persistence is enabled and a Unit of Work is available, the method now queries the database first — using get_by_namespace() when a namespace filter is given, or the new list_all() otherwise — and refreshes the in-memory cache with any newly-discovered entries. Falls back to the in-memory cache on DatabaseError or when no UoW is wired. Added ActionRepository.list_all() to retrieve all persisted actions ordered by namespace and name. Refs: #760 |
||
|
|
59be111e1d |
fix(plan): complete apply phase inline in auto_progress and lifecycle-apply CLI
Modified auto_progress() to complete the Apply phase immediately after transitioning from Execute to Apply, since Apply is a metadata transition with no LLM processing. This ensures `plan execute` drives the plan to the terminal `applied` state when the automation profile permits (ci, full-auto profiles with auto_apply < 1.0). Extracted `_complete_apply_if_queued()` helper that consolidates the Apply-completion pattern (start_apply + complete_apply) into a single method with error recovery (calls `fail_apply` on failure) and async-job guard (skips inline completion when async execution is enabled to avoid orphaning enqueued jobs). Used by `auto_progress()`, `lifecycle_apply_plan()`, and `try_auto_run()`. Added PlanLifecycleService.try_auto_run() that drives plans through all lifecycle phases (strategize → execute → apply) when automation-profile thresholds allow automatic progression. Each phase checks the profile's auto_* threshold before proceeding; a threshold of 1.0 stops the plan at that phase boundary for human approval. Fixed `lifecycle-apply` CLI leaving plans stuck in `apply/queued` without completing. The command now calls `_complete_apply_if_queued()` when the plan is in Apply/queued, driving it to the terminal `applied` state. Fixed stale RICH output in `lifecycle_apply_plan` that printed "Plan is now in Apply phase (queued)" after the plan had already reached terminal `applied` state; now branches on `plan.is_terminal`. Additional fixes: - SQLite UNIQUE constraint violation in LifecyclePlanRepository.update(): added session.flush() after clear() on child collections (project_links, arguments, invariants) before re-inserting rows - Added 'state' alias in _plan_spec_dict() JSON output for spec §Example 7 jq compatibility - Updated plan execute and lifecycle-apply reference documentation Refs: #753 |
||
|
|
00881a3e5f
|
feat(observability): implement Event System Domain Event Taxonomy (full EventType enum + DomainEvent model)
Verified and completed the Event System Domain Event Taxonomy implementation. Gap analysis: - EventType enum (38 types across 12 domains): ALREADY EXISTED, verified complete - DomainEvent Pydantic model (all 9 fields): ALREADY EXISTED, verified complete - EventBus Protocol (emit + subscribe): ALREADY EXISTED, verified complete - ReactiveEventBus (RxPY Subject, stream property): ALREADY EXISTED, verified - LoggingEventBus (structlog-based): ALREADY EXISTED, verified complete Added: - In-memory audit_log on ReactiveEventBus (list[DomainEvent] with defensive copy) - Emit ordering aligned with specification: RxPY stream push, then audit append, then handler dispatch (§Event System) - Clarified audit_log docstring: volatile in-memory log, not durable SQLite persistence (durable persistence wired separately via audit service layer) - Behave feature: features/observability/event_system_taxonomy.feature (36 scenarios) - Step definitions: features/steps/event_system_taxonomy_steps.py - Robot integration test: robot/event_system_taxonomy_integration.robot (13 test cases) - Robot helper: robot/helper_event_system_taxonomy.py (13 subcommands) - ASV benchmark: benchmarks/bench_event_bus.py (5 benchmark suites) - vulture_whitelist.py: added audit_log property Code review fixes applied: - Reordered emit() to match spec: on_next → audit_log → handlers (was: audit → on_next → handlers) - Clarified audit_log docstring to distinguish volatile in-memory log from spec SQLite table - Fixed benchmark TaxonomyAuditLogSuite: parameterized setup instead of inline construction - Added teardown to TaxonomyLoggingEmitSuite to restore logging.disable(NOTSET) - Clear _received list between benchmark iterations to reduce noise - Catch specific pydantic.ValidationError in frozen model mutation tests - Consolidated duplicate singular/plural step definitions - Tightened EventType count threshold from >=30 to >=38 Quality gates: - lint: PASSED - typecheck: PASSED (0 errors) - unit_tests: 36/36 new scenarios PASSED (pre-existing 12 flaky failures unchanged) - integration_tests: 1483/1483 PASSED - security_scan: PASSED - dead_code: PASSED ISSUES CLOSED: #587 |
||
|
|
34c2acc354 |
fix(acms): implement context tier runtime promotion/demotion/eviction
Implement the missing runtime logic for the ACMS context tier service. Previously only data models and manual promote()/demote()/evict_lru() methods existed. This commit adds: - Auto-promotion on access: get() now promotes fragments one tier up when access_count reaches the configurable promotion_threshold (default: 5 accesses). The access counter resets after each successful promotion so fragments must accumulate fresh accesses before the next tier transition. - Staleness enforcement: new enforce_staleness() method demotes hot fragments older than hot_ttl (default: 24h) to warm, and warm fragments older than warm_ttl to cold. A snapshot of existing warm-tier IDs prevents double-demotion in a single pass. - Budget enforcement on store and promote: store() and promote() now enforce TierBudget.max_tokens_hot by evicting LRU hot-tier fragments until the token budget is met. The eviction loop uses incremental token tracking to avoid recomputing the sum. - Event emission: Added TIER_PROMOTED, TIER_DEMOTED, TIER_EVICTED event types to EventType enum. All tier transitions emit DomainEvent instances through the optional EventBus. - Configuration: Added context_tier_promotion_threshold, context_tier_hot_ttl_hours, context_tier_warm_ttl_hours settings. The warm TTL setting also accepts the spec-defined env var CLEVERAGENTS_CTX_WARM_HOURS as an alias. - DI wiring: container.py now injects event_bus into context_tier_service. Review fixes applied (code review on PR #1150): - C1: Reset access_count to 0 after each auto-promotion to prevent chain promotion that bypassed the warm tier. - C2: Call _enforce_hot_budget() inside promote() warm-to-hot path so auto-promoted fragments respect the token budget. - H1: Corrected _enforce_hot_budget() docstring: actual complexity is O(n + n*k) not O(n), since min() scans remaining entries on each eviction. - M1: Added CLEVERAGENTS_CTX_WARM_HOURS as an additional env var alias for context_tier_warm_ttl_hours per specification line 30555. Review fixes applied (second code review on PR #1150): - B-CRIT-1: Fixed data loss in promote() warm-to-hot: emit TIER_PROMOTED before _enforce_hot_budget(), and if the promoted fragment is evicted by budget, restore it to the warm tier instead of silently losing it. - B-HIGH-1: Fixed self-eviction on store(): fragments whose token_count exceeds the entire hot-tier budget are now redirected to the warm tier with a warning log. - B-MED-1: Wrapped _emit_tier_event() in try/except so a failing event bus does not break tier operations (best-effort emission). - B-MED-2: Fixed event ordering so TIER_PROMOTED fires before any budget-triggered TIER_EVICTED events. - D-LOW-1: Fixed type hint in Robot helper (dict[str, Callable]). - D-LOW-2: Added __all__ export to context_tiers.py. - S-LOW-1: Added thread-safety docstring note to ContextTierService. Review fixes applied (third code review on PR #1150): - B-MED-1: Added TIER_DEMOTED event emission for oversized fragment redirect in store(), closing the observability gap where the only tier transition without event emission was the hot-to-warm redirect for fragments exceeding the entire hot-tier budget. - S-LOW-1: Added CLEVERAGENTS_CTX_HOT_HOURS as an additional env var alias for context_tier_hot_ttl_hours, for consistency with the warm-tier alias CLEVERAGENTS_CTX_WARM_HOURS. - S-LOW-2: Added docstring note to enforce_staleness() reconciling the hot-tier TTL with the specification statement that hot-tier retention is "Until resource removed" (TTL controls tier placement, not data retention). Review fixes applied (fourth code review on PR #1150): - B-HIGH-1: Reset access_count to 0 on demotion so that demoted fragments must accumulate fresh accesses before re-promotion. Without this reset, a previously popular fragment whose access_count already exceeded the promotion threshold would be re-promoted on the very next get() call, making staleness enforcement ineffective. Review fixes applied (freemo APPROVED review on PR #1150): - #1: Removed all # type: ignore annotations from test files. Fixed _EventCollector, _FailingBus, and _NullBus subscribe() signatures to use Callable[[DomainEvent], None] matching the EventBus protocol. Replaced dict-spread TieredFragment construction with explicit keyword arguments and post-construction assignment. - #2: Extracted runtime policy logic (enforce_staleness, _maybe_auto_promote, _re_fetch_after_promotion, _enforce_hot_budget, _emit_tier_event) into TierRuntimeMixin in tier_runtime.py to reduce context_tiers.py toward the 500-line guideline. - #3: Added fragment_id non-empty validation guard to promote() and demote() per CONTRIBUTING.md argument validation policy. - #8: Renamed _resolve to _re_fetch_after_promotion for clarity. Removed @tdd_expected_fail from TDD tests (Behave + Robot) as the bug is now fixed. All 3 TDD scenarios pass normally. Tests: 27 Behave scenarios (24 feature + 3 TDD), 4 Robot integration tests, 4 ASV benchmark suites. ISSUES CLOSED: #821 |
||
|
|
06130212ed |
test(e2e): workflow example 5 — database schema migration with safety nets (review profile) (#816)
## Summary E2E test for Workflow Example 5 — database schema migration with safety nets using the **review** automation profile. Exercises the full spec-aligned workflow: - **Custom resource type registration** via `resource type add --config` (postgres-db type with `transaction_rollback` sandbox strategy, `--host`, `--port`, `--database`, `--schema` CLI args with flat `type`/`default` fields per `ResourceTypeArgument` schema) - **Custom resource instantiation** — attempts `resource add` with the custom type to exercise mixed resource types, followed by `project link-resource` to link DB resource to the project - **Custom skill creation** with spec-aligned database tools: `local/query_db` (read-only), `local/execute_migration` (writes, checkpointable), `local/backfill_column` (writes, checkpointable) — registered via `skill add --config`, with namespaced tool reference names per `SkillToolRefSchema` validation - **Action creation** with `automation_profile: review`, `reusable: true`, `state: available`, spec invariants, and typed `arguments` section (`table_name`, `column_name`, `column_type`, `backfill_source` — all required per spec, using `arguments` field per `ActionConfigSchema`) - **Plan use** with `--arg` flags exercising parameterized action invocation including `backfill_source=audit_log`, plus **explicit `--automation-profile review`** flag (action-to-plan profile propagation is not yet wired in `PlanLifecycleService.use_action`) - **Phased child plan verification** via `plan tree --format json` with `decision_count >= 2` hard assertion on framework decisions plus WARN tiers for LLM decomposition quality (`< 3`, `< 5`) - **Plan phase assertion** — hard assertion that phase is populated after execute - **Checkpoint-based rollback** with hard assertions: `rc=0` on rollback success, `rc!=0` on fake checkpoint, None guard for JSON null checkpoint IDs, re-execute with Traceback/INTERNAL checks on success and explanatory comment on failure path - **Plan diff** with hard `rc=0` assertion and content-signal verification - **Migration content verification** — baseline SHA saved before apply, diff against baseline (not `HEAD~1`), WARN-level check on migration keywords (`last_login`, `schema`, `migration`, `column`, `alter`) — flexible per LLM non-determinism - **Commit count** assertion `>= 2` (fixture baseline: Create Temp Git Repo + DB fixture commit), WARN if no additional commits from lifecycle-apply - **Backfill evidence** WARN-level check in plan tree/execution output (`backfill`, `batch`, `populate`, `last_login`) with explanatory comment noting tree covers decomposition plan - **Combined AC #6 gate** — if *both* migration content *and* backfill evidence are absent, explicit WARN visibility for CI debugging - **Terminal state assertion** after `lifecycle-apply` — `plan status` call verifies phase/processing_state reflects terminal or apply-progress outcome - **Automation profile fallback verification** — if `plan use` output omits `automation_profile`, falls back to `plan status` for secondary verification (hard assertion always runs) - **Traceback and INTERNAL checks** on all CLI commands (resource add, project create, resource type add, skill add, action create, plan use, strategize, execute, plan tree, plan status, plan diff, plan rollback, re-execute after rollback, lifecycle-apply) including custom resource error paths - **Dynamic actor selection** — detects available API keys (Anthropic/OpenAI) at suite setup - **Skip If No LLM Keys** guard for graceful CI degradation - **Test-level teardown** with diagnostic logging for both plan status and plan tree on failure - **30-minute timeout** covering worst-case rollback+re-execute path - **Force Tags** for consistency with `m6_acceptance.robot` - **Timeout parameters** (`timeout=60s on_timeout=kill`) on all local `Run Process` git commands - **Sequential section numbering** (1 through 15) for readability Closes #751 ISSUES CLOSED: #751 ## Approach Follows the patterns established by `m6_acceptance.robot` and `m2_acceptance.robot`: - `WF05 Suite Setup` initialises the workspace, generates a unique run suffix, and detects available LLM API keys - `Safe Parse Json Field` from `common_e2e.resource` for JSON field extraction with None guards for JSON null values - All CLI commands use `--format json` for predictable, parseable output - `expected_rc=None` with explicit `Should Be Equal As Integers` for detailed failure messages - Hard assertions on infrastructure/framework behavior (CLI commands, phase transitions, tool registration) - WARN-level assertions on LLM-dependent output (decision decomposition, migration content, backfill evidence, commit count) — per ticket requirement "output validation is flexible" - Traceback and INTERNAL checks on all CLI commands following `m2_acceptance.robot` pattern - Baseline SHA approach for post-apply diff verification eliminates false positives from fixture commits ## Bug Fix: LifecyclePlanRepository.update() UNIQUE Constraint Violation **Root cause**: `LifecyclePlanRepository.update()` called `clear()` on child relationship collections (project_links, arguments, invariants) followed by `append()` with new items, but only flushed at the end. SQLAlchemy's default operation ordering can emit INSERTs before DELETEs within the same flush, causing `UNIQUE constraint failed: plan_arguments.plan_id, plan_arguments.name` when plans have arguments. **Fix**: Group all three `clear()` calls together and flush them before appending new rows. This ensures the DELETEs are committed before any INSERTs, preventing the UNIQUE constraint violation. **Impact**: This was a latent bug affecting ALL plans with arguments when `update()` is called. Previously undetected because existing E2E tests (M1, M2, M5, M6) create plans without `--arg` flags. ## Review Fixes (addressing medium findings from @CoreRasurae review) | # | Finding | Fix | |---|---------|-----| | **BUG-1** | No regression test for UNIQUE constraint fix | Added targeted BDD scenario in `repositories_coverage_boost.feature` — creates plan with argument `x=v1`, updates to `x=v2`, asserts no `IntegrityError` | | **TEST-1** | AC #4 weakened — fragile string counting | Replaced raw `count('"decision_id"')` with proper JSON parsing via `json.loads()`, recursive tree walking for decision counting, structural `children_key_count` and `child_link_count` verification | | **TEST-2** | AC #5 conditionally tested | Added explicit WARN log when no checkpoint_id is present ("AC #5 visibility"); fake checkpoint test now runs unconditionally (moved outside IF/ELSE) with Traceback/INTERNAL checks | | **TEST-3** | No terminal state assertion after lifecycle-apply | Added `plan status` call after apply with phase/processing_state extraction; hard assertion on terminal state or apply-phase progress | | **TEST-4** | AC #6 migration/backfill WARN-only | Added combined gate (`has_ac6_evidence`): if *both* migration and backfill evidence are absent, explicit WARN for CI visibility. WARN-only is intentional per ticket AC "output validation is flexible" | | **TEST-5** | Automation profile silently skipped | Added fallback to `plan status --format json` when `plan use` output omits `automation_profile`; hard assertion (`Should Be Equal As Strings review`) now always executes | | **TEST-8** | Missing Traceback/INTERNAL on custom resource error paths | Added Traceback/INTERNAL checks inside both `resource add` and `project link-resource` ELSE branches with `NoSuchOption` guard | ## Quality Gates - `nox -e lint` ✅ - `nox -e typecheck` ✅ (0 errors) - `nox -e unit_tests` ✅ (471 features, 12,422 scenarios, 0 failures) - `nox -e integration_tests` ✅ (1,727 tests, 0 failures) - `nox -e e2e_tests` ✅ (42 tests, 42 passed, 0 failed) - `nox -e coverage_report` ✅ (98%, meets threshold) ## Manual Verification ### Prerequisites - `OPENAI_API_KEY` or `ANTHROPIC_API_KEY` environment variable set ### Commands ```bash nox -e e2e_tests # Or run just this suite: python -m robot --outputdir build/reports/robot --include E2E robot/e2e/wf05_db_migration.robot ``` Reviewed-on: cleveragents/cleveragents-core#816 Reviewed-by: Luis Mendes <luis.mendes@cleverthis.com> Co-authored-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me> Co-committed-by: Jeffrey Phillips Freeman <the@jeffreyfreeman.me> |
||
|
|
c082c8f022 |
fix(cli): bypass migration prompt entirely when --yes flag is passed
Make require_confirmation configurable on UnitOfWork so that when the --yes flag is passed to agents init, the migration runner skips the confirmation prompt entirely rather than calling a prompt that always returns True. The previous approach (injecting a prompt callback that auto-approves) was a workaround that left the require_confirmation=True hardcoded in _ensure_database_initialized, meaning the prompt code path was still exercised unnecessarily. Changes: - Add require_confirmation parameter to UnitOfWork constructor (default True for backward compatibility) - Pass self._require_confirmation to MigrationRunner.init_or_upgrade instead of hardcoded True - Update init_command to pass require_confirmation=False when --yes is set, with the prompt callback retained as belt-and-suspenders The TDD bug-capture tests from #842 (features/tdd_init_yes_no_input.feature, robot/tdd_init_yes_no_input.robot) now run as normal regression tests with @tdd_expected_fail already removed. All nox quality gates pass: - lint, typecheck: clean - unit_tests: 12230 scenarios passed - integration_tests: all passed - coverage_report: 98.38% (threshold >97%) ISSUES CLOSED: #783 |
||
|
|
3cf3f1f69e |
feat(validation): implement Fix-then-Revalidate orchestration loop for required validations
Implemented FixThenRevalidateOrchestrator with full diagnosis → self-fix → re-validation → retry limit → strategy revision → user escalation → terminal failure flow. Added retry counting per validation per plan, configurable retry limit (default 3), auto_strategy_revision flag support, and validation_fix_history recording in plan execution metadata. Review fixes applied (round 1): - Added try/except around fix_callback and revalidate_callback (spec: validation errors treated as required failure regardless of mode) - Wired optional EventBus for VALIDATION_FIX_ATTEMPTED/SUCCEEDED/EXHAUSTED events - Fixed total_attempts derivation from fix_history length instead of cumulative retry counts - Updated module docstring to reflect implemented vs caller-responsible steps - Added exhausted-retry logging on re-invocation - Replaced bare assert in Robot helper with explicit _check() function - Added 10 new Behave scenarios covering exception paths, multi-failure, reset, field validation, boundary values, event_bus property, and exhausted re-invocation Review fixes applied (round 2 — PR #711): - B1+B7: Fixed TOCTOU race in _fix_single_validation; atomic claim-per-iteration under RLock; eliminated defaultdict auto-vivification with .get() reads - B2: Added validation_name mismatch check on revalidate_callback return - B3: Added fix_description truncation to 2000 chars before FixAttemptRecord - T1: Switched threading.Lock to threading.RLock for defensive reentrancy - A1: Added model_validator preventing escalated+terminal_failure both True - R1: Added max_length=255 to FixAttemptRecord.validation_name - S7: Fixed misleading docstring about automation_profile integration - D4: Fixed timestamp field description to specify UTC - D1: Renamed misleading benchmark time_fix_after_two_retries - Added 5 new Behave scenarios for truncation, model constraints, name mismatch ISSUES CLOSED: #583 |
||
|
|
e52c79e958 |
test: add TDD bug-capture test for #1024 — SQLite DB URL CWD resolution
Add Behave BDD scenarios (tagged @tdd_bug @tdd_bug_1024 @tdd_expected_fail) that verify the default database_url resolves inside CLEVERAGENTS_HOME rather than the current working directory. Two scenarios exercise both the container get_database_url() helper and the Settings model default. Add Robot Framework integration tests with a helper script exercising the same resolution paths via subprocess, verifying database URL resolution and CLI database file placement relative to CLEVERAGENTS_HOME. The tests currently fail as expected because the bug in #1024 is still present: the relative SQLite path sqlite:///cleveragents.db resolves against CWD. The @tdd_expected_fail tag inverts the result so CI passes. ISSUES CLOSED: #1034 |
||
|
|
93da31e80f |
fix(skill): resolve skill add persistence regression after PR #640
Root cause: _build_skill_service in container.py created a SkillRepository pointing at the database but did not ensure the skills/skill_items tables existed. When the tables were missing, SkillRepository.list_all() and create() failed silently (caught by SkillService._load_from_db and _persist_skill exception handlers), causing the service to operate in in-memory-only mode. Skills added in one CLI process were lost when a new process created a fresh SkillService. Additionally, SkillRepository lacked auto_commit support. Each call to session_factory() returned a new session, so the flush in create/update/ delete operated on a different session than the commit in SkillService._commit(), meaning data was never actually persisted even when the tables existed. Fix: 1. Add targeted table creation in _build_skill_service (following the pattern in _build_session_service) — checks for missing skills and skill_items tables and creates them via Base.metadata.create_all. 2. Add auto_commit parameter to SkillRepository (following the pattern in SessionRepository) so each mutating method commits and closes its own session. 3. Pass auto_commit=True from the container builder. 4. Remove @tdd_expected_fail from TDD test (leaving @tdd_bug and @tdd_bug_980 as permanent regression guards). ISSUES CLOSED: #980 |
||
|
|
9e316b1a3e |
fix(domain): align plan lifecycle model validation with specification
Aligned the plan lifecycle model with the specification: 1. ERRORED is now treated as terminal in is_terminal property, matching the spec table where errored is marked "Terminal? Yes" for all processing phases. 2. Added per-phase state validation via model_validator: APPLIED and CONSTRAINED are only valid in APPLY phase; COMPLETE is only valid in STRATEGIZE or EXECUTE phases. Invalid combinations now raise ValueError at construction time. 3. Updated ProcessingState.COMPLETE docstring to clarify phase-level terminality semantics. 4. Fixed assignment ordering in execute_plan() to set processing_state before phase, consistent with the state-first pattern used in apply_plan() and _perform_reversion(). 5. Added defensive coercion in LifecyclePlanModel.to_domain() to handle legacy DB rows with invalid phase/state combinations (e.g. APPLY/COMPLETE -> APPLY/APPLIED) with warning-level logging for observability. 6. Updated module docstrings: ERRORED description now reflects terminal semantics, terminal outcomes location clarified for all phases, can_revert_to docstring notes ERRORED/CONSTRAINED are terminal but revertable, is_terminal docstring explains the distinction between terminal and permanently irrecoverable and documents why COMPLETE is not plan-terminal despite the spec marking it "Terminal? Yes" (phase-level vs plan-level). 7. Updated PlanResumeService.validate_eligibility() docstring to reflect that ERRORED is now terminal but still eligible for resume. 8. Added CHANGELOG entry. ISSUES CLOSED: #918 |
||
|
|
8a87262f86 |
feat(sandbox): implement overlay filesystem sandbox strategy (#994)
## Summary Implement the overlay filesystem sandbox strategy with OverlayFS support and userspace fallback. ### Implementation **`OverlaySandbox`** (`infrastructure/sandbox/overlay.py`, 497 lines): - Detects OverlayFS availability at runtime via `/proc/filesystems` + `os.geteuid() == 0` check - **Real OverlayFS mode** (requires root): creates upper/work/merged dirs, mounts overlay filesystem, captures writes in upper layer - **Userspace fallback** (default in CI/containers): `shutil.copytree` the original into merged dir, tracks changes via `filecmp` diff on commit - `create()`: sets up directory structure, mounts if available - `commit()`: copies changed/added files from overlay to original, removes deleted files - `rollback()`: unmounts (or removes) merged, recreates from scratch - `cleanup()`: unmounts, removes all temp dirs, idempotent ### Domain Model Updates - Added `OVERLAY = "overlay"` to `SandboxStrategy` enum in both `resource_type.py` and `resource.py` - Added `STRATEGY_OVERLAY` to `SandboxFactory`, registered for `fs-mount`, `fs-directory`, `fs-file` resources ### Tests - **22 Behave scenarios**: full lifecycle (create/commit/rollback/cleanup), status transitions, path traversal guard, fallback detection, error handling - **6 Robot integration tests**: end-to-end overlay sandbox operations ### Quality Gates | Session | Result | |---|---| | `nox -s lint` | PASS | | `nox -s typecheck` | PASS (0 errors) | | `nox -s unit_tests` | PASS (10,917 scenarios) | | `nox -s coverage_report` | 97% (>= 97%) | Closes #880 Reviewed-on: cleveragents/cleveragents-core#994 Co-authored-by: Brent Edwards <brent.edwards@cleverthis.com> Co-committed-by: Brent Edwards <brent.edwards@cleverthis.com> |
||
|
|
051ee7c290 |
test(coverage): add Behave BDD tests to improve coverage across 52 source files
Added 52 new .feature files and corresponding _steps.py files targeting previously uncovered code paths in the following areas: - TUI layer: app, commands, persona (state/schema/registry), widgets, input (shell_exec, reference_parser) - Application services: plan lifecycle/service/executor, session, project, repo indexing, correction, checkpoint, actor, llm_actors, strategy coordinator, resource file watcher, service retry wiring - CLI commands: session, resource, repl, plan, db, automation_profile - Domain models: retry_policy, resource_type, cost_budget, docker_compose_analyzer, detail_level, _sql_string_aware, _postgresql_helpers - Core: circuit_breaker, retry_service_patterns - Infrastructure: repositories, transaction_sandbox, strategy_registry, plugins/loader, container - Config: settings - Agents: plan_generation, context_analysis, auto_debug - A2A: facade All new tests follow the Behave/Gherkin BDD standard. Resolved step definition collisions with unique prefixes. Fixed Alembic fileConfig logger disabling issue (disable_existing_loggers=False). ISSUES CLOSED: #1068 |
||
|
|
399939a6db |
refactor(db): migrate from create_all() to Alembic-managed schema migrations
Complete the Alembic migration infrastructure by adding CLI commands, improving stamp logic, and adding comprehensive lifecycle tests. Key changes: - Added agents db CLI command group (db.py) with 5 subcommands: migrate (autogenerate), upgrade, downgrade, current, history. All delegate to MigrationRunner which wraps Alembic command API. - Registered the db command group in main.py CLI registration. - Fixed legacy database stamp logic in MigrationRunner to stamp at "head" instead of "001_initial_schema" when pre-Alembic tables are detected. This avoids migration failures when create_all-produced tables already exist (migrations would try to CREATE TABLE and fail with "table already exists"). - Added commit() after stamp to ensure alembic_version is persisted before subsequent operations on the same in-memory database. - Added FakeConnection.commit() method to the mock test infrastructure to support the new commit call in the stamp path. - Added Behave feature (db_migration_lifecycle.feature) with 8 scenarios covering: forward migration, rollback, round-trip, CLI upgrade/current/downgrade, legacy stamp logic, and init_database schema validation. - Added vulture whitelist entries for new CLI commands. Note: init_database() still uses Base.metadata.create_all() as the primary schema creation path. Full migration to Alembic-only init is deferred until ORM model constraints are reconciled with migration scripts (action_arguments UniqueConstraint mismatch). ISSUES CLOSED: #941 |
||
|
|
48ecf4c00c |
fix(cli): add --execution-env-priority flag to plan use (#972)
## 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>
|
||
|
|
09d3b0aa20
|
style: fix style and type checks | ||
|
|
2acb957d83
|
fix(cli): replace in-memory automation-profile repository with database-backed persistence
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 |
||
|
|
2688c85769 |
feat(extensibility): implement Custom Sandbox Strategy Registration via SandboxStrategy Protocol
Implement SandboxStrategyProtocol, a 9-method @runtime_checkable Protocol enabling third-party sandbox strategy registration. Includes: - SandboxStrategyProtocol with create/read/write/diff/commit/rollback/checkpoint/ restore_checkpoint/cleanup methods - SandboxRef (frozen dataclass) and DiffView/DiffEntry (Pydantic models) - SandboxStrategyRegistry with config-driven registration, Protocol validation, thread safety, and clear/list/has operations - BuiltInSandboxStrategyAdapter wrapping existing Sandbox implementations to conform to the new Protocol - CustomStrategyConfig for YAML/dict-based strategy registration - SandboxFactory integration with custom_registry parameter, has_custom_strategy() and get_custom_strategy_class() - 25 Behave BDD scenarios (85 steps) covering protocol, registry, adapter, config, and factory integration - 8 Robot Framework integration tests with real filesystem operations - ASV benchmarks for registry and adapter operations - Developer documentation ISSUES CLOSED: #586 |
||
|
|
9ef8502570 | fix(database): reset session factory after engine disposal in init_database | ||
|
|
eb770643c2
|
test(e2e): validate M5 acceptance criteria for v3.4.0 milestone closure
Add four CLI-based integration test cases to the M5 E2E verification suite that exercise the exact commands from the v3.4.0 milestone description via real subprocess calls to `python -m cleveragents`. CLI test cases: - CLI Project Create Large Project: `agents project create local/large-project` - CLI Resource Add Git Checkout: `agents resource add git-checkout ...` - CLI Project Link Resource: `agents project link-resource ...` - CLI Project Show Displays Linked Resource: `agents project show ...` Each test creates an isolated temp directory with its own CLEVERAGENTS_HOME, initialises a workspace via `agents init`, runs the target CLI command as a subprocess with `on_timeout=kill`, asserts zero exit code and verifies expected output, then tears down the temp directory. Bug fix: ProjectResourceLinkRepository.create_link() and remove_link() only called session.flush() without session.commit(), causing linked resource data to be silently lost between sessions. Added session.commit() to both methods, plus finally: session.close() to match the session-factory lifecycle pattern used by all other mutating repository methods. Added session.refresh() and session.expunge() before return in create_link() so the returned ORM instance is fully loaded and usable in detached state after session close. Regression guard: Added two cross-session persistence Behave scenarios (project_repository.feature) that open a new session from the same engine after create_link/remove_link and verify the operation was durably committed. Test improvements from review feedback (rounds 1 and 2): - Per-test CLEVERAGENTS_HOME isolation via env: override on all Run Process calls to prevent shared state between tests. - Stronger ULID-based assertions: CLI tests capture the resource_id ULID from resource show output and verify the exact ID in project show output. - Regex-based branch assertion (branch.*main) instead of generic string match. - Test 4 differentiated from Test 3 by verifying resource show independently after linking and asserting the specific resource ULID. - Documentation noting context tier and ACMS criteria are validated at the Python API level via helper_m5_e2e_verification.py. - Updated ProjectResourceLinkRepository class docstring documenting commit and close behaviour of mutating methods. - CHANGELOG entry for both test additions and production bug fix. - Redundant Library imports removed, --format plain on project list, comments explaining fake repo directories, standardised Run Process line style. Context/ACMS CLI coverage is intentionally not added because the CLI does not yet expose dedicated context/ACMS inspection commands. These criteria are validated at the Python API level via helper_m5_e2e_verification.py. ISSUES CLOSED: #496 |
||
|
|
3e3e9b4b5d
|
feat(observability): wire AuditService.record() into domain services via EventBus auto-dispatch
Implements AuditEventSubscriber that subscribes to all 9 security-relevant EventType members and persists redacted audit entries via AuditService.record(). Key components: - AuditEventSubscriber: bridges EventBus and AuditService (SEC7) - SECURITY_EVENT_MAP: maps EventType enum to audit type strings - Redaction via redact_dict() on event details before persistence - Graceful error handling: failures logged, never propagated Post-review fixes applied: - BUG-1: Remove dead correlation_id null-check guard (DomainEvent.correlation_id is always non-None via ULID default_factory) - SEC-2: Redact exception messages in warning logs via redact_value() to prevent potential leakage of sensitive internal state (e.g. DB connection strings) - PERF-3: Pre-generate unique DomainEvent instances in ASV benchmark setup to avoid skew from reusing a single frozen object - Wire event_bus from the DI container into CorrectionService (plan.py), ConfigService (config.py, skill.py x2, server.py), and PersistentSessionService (session.py) at their CLI construction sites. Closes #581 |
||
|
|
76fb84c3d0 |
Merge remote-tracking branch 'origin/master' into bugfix/m3-session-list-error
# Conflicts: # CHANGELOG.md |
||
|
|
e732c32981 |
fix(cli): handle missing database in session list command
Register PersistentSessionService in the DI Container so that 'agents session list' (and all other session subcommands) no longer throw AttributeError due to a missing 'db' provider. Changes: - Add _build_session_service() factory and session_service provider to Container, with targeted table creation for session/session_messages only (avoids bypassing Alembic for the full schema). - Add auto_commit parameter to SessionRepository and SessionMessageRepository; when True each method commits and closes its own database session, preventing resource leaks in CLI context. - Rewrite _get_session_service() to resolve via container.session_service() with module-level caching. - Add (DatabaseError, AttributeError) error handling with logging to all seven session subcommands (list, create, show, delete, export, import, tell). - Remove @tdd_expected_fail tags from all session test files so they run as proper regression tests. ISSUES CLOSED: #554, #570, #680 |
||
|
|
7ac3f1352c
|
feat(devcontainer): add container-aware tool execution and I/O forwarding
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 |
||
|
|
c054675167 |
feat(resource): add database resources
Implement database resource types (postgres, mysql, sqlite, duckdb) with connection args, auth handling, and transaction-based sandbox strategy using BEGIN/ROLLBACK/COMMIT wrappers. Key changes: - Add DatabaseResourceHandler with 4 database type definitions - Implement TransactionSandbox for transaction_rollback strategy - Wire TransactionSandbox into SandboxFactory - Register database types in bootstrap_builtin_types - Add connection validation with credential masking - Add Behave BDD tests, Robot integration tests, ASV benchmarks ISSUES CLOSED: #342 |
||
|
|
958eb0c060 |
feat(observability): implement Metrics Collection Framework (14 metric types with Histogram/Counter/Gauge)
Implement the structured metrics collection framework covering 14 operational metric types with proper Histogram, Counter, and Gauge semantics per the spec (Architecture > Observability > Metrics Collection, lines ~43805-43825). Domain layer: - Add MetricType enum (HISTOGRAM, COUNTER, GAUGE) to metrics.py - Add MetricDefinition model and METRIC_DEFINITIONS registry mapping all 14 OperationalMetricKey values to their metric types - Extend MetricCollector with typed factory methods (histogram, counter, gauge) and 14 convenience methods (plan_duration, plan_cost, plan_decision_count, subplan_count, actor_invocation_count, actor_latency, tool_invocation_count, tool_error_rate, context_build_time, context_token_count, llm_call_count, llm_total_tokens, llm_total_cost, llm_avg_latency) - Extend MetricEntry with optional metric_type field that auto-resolves from METRIC_DEFINITIONS via MetricCollector.record() Infrastructure layer: - Add MetricsEmitter (infrastructure/observability/metrics_emitter.py) with emit(), emit_batch(), from_settings(), and enabled/disabled support for structured log emission in local mode - Add metrics_log_processor (config/metrics_processor.py) for structlog integration Configuration: - Add metrics_enabled and metrics_export_prometheus settings - Register MetricsEmitter as DI Singleton in application container Instrumentation: - Add best-effort metric emission in PlanExecutor for plan_duration (both runtime and stub execute paths) and plan_decision_count (strategize path) via _try_emit_metric helper that tolerates invalid plan IDs in test fixtures Testing: - 34 Behave BDD scenarios (features/observability/metrics_collection.feature) - 8 Robot Framework integration tests (robot/metrics_collection.robot) - ASV benchmark suite (benchmarks/bench_metrics_collection.py) Closes #579 |
||
|
|
521a552e56 |
feat(extensibility): implement Plugin Architecture Framework with module:ClassName resolution
Implement plugin architecture framework enabling custom implementations of tools, strategies, sandbox strategies, index backends, and pipeline components through module:ClassName resolution and entry point discovery. Key changes: - Add infrastructure/plugins/ package with PluginLoader, PluginManager, types, exceptions - Implement dynamic import via module:ClassName with security prefix allowlist - Add entry point discovery via importlib.metadata - Add Protocol validation for loaded plugin classes - Add plugin lifecycle management (discover/activate/deactivate) - Add config-driven registration (custom_module + custom_class + custom_options) - Register PluginManager in DI container - Add comprehensive Behave BDD tests, Robot integration tests, ASV benchmarks ISSUES CLOSED: #585 |
||
|
|
bb8175aa11 |
feat(resource): add resource type inheritance and polymorphic tool matching
Implement ADR-042 single-inheritance for resource types with polymorphic tool and handler resolution. Core changes: - Add `inherits` field to ResourceTypeConfigSchema, ResourceTypeSpec, and ResourceTypeModel with chain depth validation (max 5), circular inheritance detection, and built-in-from-custom guard - New inheritance.py module (390 lines): resolve_inheritance_chain(), validate_chain(), is_subtype_of(), resolve_fields(), find_subtypes() - Wire inheritance into ResourceRegistryService: chain validation on register_type(), resolve_type_inheritance_chain(), is_subtype_of() - Add find_tools_for_resource() to ToolRegistry with polymorphic matching that walks the inheritance chain - Add resolve_handler_polymorphic() to handler resolver - Alembic migration m6_004_resource_type_inherits adds inherits column and index to resource_types table CLI changes: - `agents resource type list` shows Inherits column - `agents resource type show` displays inheritance chain Tests: - 30 BDD scenarios in resource_type_inheritance.feature (all pass) - 18 Robot Framework test cases - 14 ASV benchmark timing methods across 4 suites Docs: - docs/reference/resource_type_inheritance.md (full reference) - Updated docs/schema/resource_type.schema.yaml - CHANGELOG entry for #513 ISSUES CLOSED: #513 |
||
|
|
7d958121ad |
feat(context): add repo indexing service
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 |
||
|
|
16dc4ab18d |
fix(project): show created project after creation (#593)
## Summary Fix `agents project show` not finding a project immediately after creation. Extends the `session.commit()` fix from #589 to also cover `update()` and `delete()` in `NamespacedProjectRepository`. ## Changes **Production fix** (`src/cleveragents/infrastructure/database/repositories.py`): - Add `session.commit()` to `create()`, `update()`, and `delete()` methods - Add `finally: session.close()` guard to all three methods - Update class docstring to reflect commit-per-method pattern **Tests & benchmarks**: - 3 Behave BDD regression scenarios (`features/project_show_after_create.feature`) - Robot Framework integration smoke tests with "not found" assertion (`robot/project_show_after_create.robot`) - ASV benchmarks for create-then-show round-trip (`benchmarks/project_show_after_create_bench.py`) ## Review feedback addressed - **F1**: Removed unrelated em-dash CHANGELOG edits — wrote clean entry from scratch - **F2**: Kept Suite Setup/Teardown (required for `${PYTHON}` variable); updated stale docs - **F3**: Added "not found" string assertion to Robot negative test case - **F4**: Removed redundant `Base.metadata.create_all()` from `_make_fresh_repo()` helper - Updated all stale TDD "expected to fail" comments — this PR includes the fix ## Process - Single squashed commit, rebased onto `master` (no merge commits) - Prescribed commit message from issue #590 metadata ISSUES CLOSED: #590 Reviewed-on: cleveragents/cleveragents-core#593 Reviewed-by: Rui Hu <rui.hu@cleverthis.com> Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com> Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com> |
||
|
|
26de6bb7de |
fix(project): persist project to database during creation (#591)
## Summary
Fix `agents project create` not persisting projects to the database. `NamespacedProjectRepository.create()` called `session.flush()` but never `session.commit()`, so projects were invisible to subsequent `agents project list` invocations that open a separate session.
## Changes
**Production fix** (`src/cleveragents/infrastructure/database/repositories.py`):
- Replace `session.flush()` with `session.commit()` in `NamespacedProjectRepository.create()`
- Add `finally: session.close()` guard for proper session lifecycle
**Tests & benchmarks**:
- 4 Behave BDD regression scenarios (`features/project_create_persist.feature`)
- Robot Framework integration smoke tests (`robot/project_create_persist.robot`)
- ASV benchmarks for create-then-list round-trip (`benchmarks/project_create_persist_bench.py`)
## Review feedback addressed
- **H3**: Added `finally: session.close()` to `create()` method
- **M1**: Removed redundant `session.flush()` before `session.commit()`
- **M2**: Updated stale TDD "expected to fail" comments — this PR includes the fix
- **M4**: Rewrote CHANGELOG entry to describe the fix, not just tests
- **F1**: Updated Robot documentation (Suite Setup/Teardown kept — needed for `${PYTHON}`)
- **F2**: Fixed bare assertion `"my-app"` to `"local/my-app"` in namespace scenario
- **F3**: Removed redundant `Base.metadata.create_all()` from `_make_fresh_repo()`
## Process
- Single squashed commit, rebased onto `master` (no merge commits)
- Prescribed commit message from issue #589 metadata
ISSUES CLOSED: #589
Reviewed-on: cleveragents/cleveragents-core#591
Reviewed-by: Rui Hu <rui.hu@cleverthis.com>
Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
|
||
|
|
583e6b7ea2
|
feat(correcting-plans): implement Predictive Error Prevention (Layer 4) with Error Pattern Database
Implement spec-mandated Layer 4 Predictive Error Prevention system: - ErrorPattern domain model with pattern text, historical failures, preventive checks, frequency tracking, and keyword-based matching. - ErrorPatternRepository with in-memory CRUD + context-matching query. - ErrorPatternService with record_failure(), match_patterns(), and get_statistics() methods. - Wire into plan execution via plan_lifecycle_service pre-execution hook. - Add error pattern statistics to CLI diagnostics output. Behave BDD: 11 scenarios covering recording, matching, formatting, stats. Robot Framework: 3 integration smoke tests. ASV benchmarks: pattern matching performance. ISSUES CLOSED: #571 |
||
|
|
d3cc7d30d7 |
fix(tool): persist tool registration to database after add
ToolRegistryRepository.create(), .update(), and .delete() called session.flush() but never session.commit(). The CLI factory creates a raw sessionmaker without a UnitOfWork wrapper, so the transaction was never committed and SQLAlchemy performed an implicit rollback when the session was garbage-collected. The same bug existed in ValidationAttachmentRepository.attach() and .detach(). Changes: - Add session.commit() after session.flush() in all five mutating methods across ToolRegistryRepository and ValidationAttachmentRepository. - Add finally: session.close() to guarantee session cleanup regardless of success or failure. - Update class docstrings to reflect the new commit-on-write semantics. - Add Behave BDD feature (tool_add_persist.feature) with scenarios for single-tool round-trip, multi-tool persistence, and duplicate rejection, using file-based SQLite to reproduce the cross-session issue. - Add Robot Framework integration test (tool_add_persist.robot) with add-then-list and fresh-list-empty scenarios. - Add ASV benchmark (tool_add_persist_bench.py) with track_list_after_add_count metric. Key decisions: - File-based SQLite (not in-memory) is used in tests because the bug only manifests when the session/engine is fully disposed between add and list, simulating separate CLI invocations. - Step patterns are prefixed with "tool-persist" to avoid AmbiguousStep collisions with existing tool_registry_steps.py. - The commit-in-repository approach was chosen over adding a UnitOfWork to the CLI factory because the CLI commands are simple CRUD operations that should auto-persist without requiring callers to remember to commit. ISSUES CLOSED: #621 |
||
|
|
3d2b138379
|
Merge branch 'master' into feature/m6plus-event-bus | ||
|
|
837ff4217b
|
feat(async): add async command execution and workers
- 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
|
||
|
|
5935940276 |
feat(sandbox): implement sandbox boundary algebra and domain computation
Implement sandbox_boundary(r) function that walks up containment edges in the resource DAG to the nearest sandboxable ancestor, enabling resources sharing a boundary to share one sandbox instance. Changes: - Add boundary.py: is_sandbox_boundary(), sandbox_boundary(), compute_sandbox_domains(), BoundaryCache (thread-safe, per-execution) - Update SandboxManager: resolve_sandbox_key() and get_or_create_sandbox_for_resource() key by (plan_id, boundary_id) instead of (plan_id, resource_id); boundary cache lifecycle methods - Define "sandboxable" via ResourceCapabilities.sandboxable + non-none sandbox_strategy as per specification section 24659-24674 - Export new symbols from sandbox __init__.py - Add vulture whitelist entries for new public API Tests: - 26 Behave BDD scenarios (features/sandbox_boundary_algebra.feature) - 5 Robot Framework integration tests (robot/sandbox_boundary_algebra.robot) - ASV benchmarks for boundary walk, domain grouping, and cache performance ISSUES CLOSED: #548 |
||
|
|
3f14cbbf7e
|
feat(observability): add LLMTrace model and operational metrics
Add LLMTrace Pydantic v2 domain model with all required fields (trace_id, plan_id, decision_id, actor, provider, model, prompt_tokens, completion_tokens, cost_usd, latency_ms, tool_calls, context_hash, streaming, retry_count, error). Define 14 OperationalMetricKey values (PLAN_DURATION_MS, PLAN_TOTAL_COST_USD, PLAN_DECISION_COUNT, ACTOR_INVOCATION_COUNT, ACTOR_LATENCY_MS, TOOL_INVOCATION_COUNT, TOOL_ERROR_RATE, CONTEXT_BUILD_TIME_MS, CONTEXT_TOKEN_COUNT, LLM_CALL_COUNT, LLM_TOTAL_TOKENS, LLM_TOTAL_COST_USD, LLM_AVG_LATENCY_MS, SUBPLAN_COUNT) with MetricEntry model and MetricCollector. Add llm_traces database table (LLMTraceModel) with LLMTraceRepository for persistence. TraceService provides recording, querying, metric computation, plan lifecycle hooks, and optional LangSmith forwarding when LANGCHAIN_TRACING_V2=true. Wired into DI container as trace_service. Includes 28 Behave BDD scenarios, 6 Robot Framework smoke tests, 3 ASV benchmark suites, and reference documentation. Fix pre-existing cli_core server_mode test flake by mocking resolve_server_mode in test steps to avoid stale config file interference. Closes #500 |
||
|
|
5bd7a507b8
|
Merge branch 'master' into feature/m6plus-event-bus |