Commit Graph

3 Commits

Author SHA1 Message Date
brent.edwards bc6a41deb6 tdd(cli): prevent actor list from triggering database updates (#1151)
## Summary

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

## Root Cause

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

## Changes

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

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

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

## Quality Gates

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

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

Closes #797

Reviewed-on: cleveragents/cleveragents-core#1151
Reviewed-by: Jeffrey Phillips Freeman <jeffrey.freeman@cleverthis.com>
Co-authored-by: Brent E. Edwards <brent.edwards@cleverthis.com>
Co-committed-by: Brent E. Edwards <brent.edwards@cleverthis.com>
2026-03-28 05:27:14 +00:00
Brent E. Edwards 73d5552467 fix(actor): handle empty actor list without validation error
ActorRegistry._actor_name() built names via f"{provider}/{model}", which
produced names with multiple slashes when providers included models
containing "/" (e.g. OpenRouter's "anthropic/claude-sonnet-4-20250514").
The resulting name violated the spec pattern ^[a-z0-9_-]+/[a-z0-9_-]+$
and triggered a ValidationError during actor upsert.

Now sanitises both provider and model components by replacing "/" with "-"
and lowercasing, so multi-slash provider models no longer break actor
listing.

Includes 6 Behave BDD regression scenarios (covering zero-provider,
multi-slash, consecutive-slash, leading-slash, and name-validation
cases), Robot Framework integration smoke tests, and ASV benchmarks.

ISSUES CLOSED: #592
2026-03-10 23:11:22 +00:00
freemo b2e923173f perf(tests): consolidate 141 trivially small feature files into 34 domain groups
Consolidate 141 BDD feature files that each complete in under 0.1 seconds
into 25 domain-grouped feature files, reducing subprocess count from 339
to ~223. Each consolidated file groups scenarios from the same domain/module
that share step definitions and fixtures. All scenarios are preserved with
clear comment headers indicating their original source file.

This reduces subprocess overhead by ~116 invocations (141 original files
replaced by 25 consolidated files), targeting the 42% of subprocess count
that contributed only 0.2% of actual test runtime.

ISSUES CLOSED: #485
2026-03-02 14:56:13 +00:00