TDD: agents actor list should not cause an update of the database (bug #797) #841

Closed
opened 2026-03-13 21:15:36 +00:00 by freemo · 4 comments
Owner

Background and Context

Bug #797 (agents actor list should not cause an update of the database) identifies a defect where running agents actor list on a fresh checkout triggers pending database migration prompts. The list command should be read-only and not trigger database updates. Per the mandatory TDD bug-fix workflow (CONTRIBUTING.md §Bug Fix Workflow), a failing test must be written before the fix is implemented.

This issue tracks the TDD test-writing phase for bug #797.

Current Behavior

No test exists that verifies agents actor list does not prompt for or apply database migrations.

Expected Behavior

A Behave BDD scenario and/or Robot Framework test tagged with @tdd_expected_fail, @tdd_bug, and @tdd_bug_797 should exist that:

  1. Sets up a fresh environment with pending migrations
  2. Runs agents actor list
  3. Asserts no migration prompt appears in the output
  4. Asserts the database schema is not modified
  5. Currently fails (proving the bug exists), but passes CI via the @tdd_expected_fail inversion

Acceptance Criteria

  • At least one Behave scenario in features/tdd_actor_list_no_db_update.feature
  • Scenario tagged with @tdd_expected_fail @tdd_bug @tdd_bug_797
  • At least one Robot Framework test in robot/tdd_actor_list_no_db_update.robot
  • Test tagged with tdd_expected_fail, tdd_bug, tdd_bug_797
  • Full test suite passes (nox -s unit_tests integration_tests)
  • ruff check and ruff format pass

Metadata

  • Commit message: test(cli): TDD failing tests for actor list database update (bug #797)
  • Branch name: tdd/m3-actor-list-no-db-update
  • Type: Testing
  • Priority: Critical
  • MoSCoW: Must have
  • Points: 2
  • Milestone: v3.2.0

Subtasks

  • Write Behave scenario exercising agents actor list with pending migrations
  • Write Robot Framework test exercising same scenario
  • Apply @tdd_expected_fail tags
  • Verify tests fail (bug present) but CI passes (inversion working)

Definition of Done

  • TDD tests exist and are tagged correctly
  • Tests demonstrate the bug (fail without @tdd_expected_fail)
  • Full CI suite passes with the inversion
  • PR merged to master
## Background and Context Bug #797 (`agents actor list` should not cause an update of the database) identifies a defect where running `agents actor list` on a fresh checkout triggers pending database migration prompts. The list command should be read-only and not trigger database updates. Per the mandatory TDD bug-fix workflow (CONTRIBUTING.md §Bug Fix Workflow), a failing test must be written **before** the fix is implemented. This issue tracks the TDD test-writing phase for bug #797. ## Current Behavior No test exists that verifies `agents actor list` does not prompt for or apply database migrations. ## Expected Behavior A Behave BDD scenario and/or Robot Framework test tagged with `@tdd_expected_fail`, `@tdd_bug`, and `@tdd_bug_797` should exist that: 1. Sets up a fresh environment with pending migrations 2. Runs `agents actor list` 3. Asserts no migration prompt appears in the output 4. Asserts the database schema is not modified 5. Currently **fails** (proving the bug exists), but passes CI via the `@tdd_expected_fail` inversion ## Acceptance Criteria - [x] At least one Behave scenario in `features/tdd_actor_list_no_db_update.feature` - [x] Scenario tagged with `@tdd_expected_fail @tdd_bug @tdd_bug_797` - [x] At least one Robot Framework test in `robot/tdd_actor_list_no_db_update.robot` - [x] Test tagged with `tdd_expected_fail`, `tdd_bug`, `tdd_bug_797` - [x] Full test suite passes (`nox -s unit_tests integration_tests`) - [x] `ruff check` and `ruff format` pass ## Metadata - **Commit message**: `test(cli): TDD failing tests for actor list database update (bug #797)` - **Branch name**: `tdd/m3-actor-list-no-db-update` - **Type**: Testing - **Priority**: Critical - **MoSCoW**: Must have - **Points**: 2 - **Milestone**: v3.2.0 ## Subtasks - [x] Write Behave scenario exercising `agents actor list` with pending migrations - [x] Write Robot Framework test exercising same scenario - [x] Apply `@tdd_expected_fail` tags - [x] Verify tests fail (bug present) but CI passes (inversion working) ## Definition of Done - TDD tests exist and are tagged correctly - Tests demonstrate the bug (fail without `@tdd_expected_fail`) - Full CI suite passes with the inversion - PR merged to `master`
freemo added this to the v3.2.0 milestone 2026-03-13 21:15:58 +00:00
Author
Owner

Dependency (TDD workflow): This TDD test issue blocks bug fix #797. This issue must be completed and merged first so the @tdd_expected_fail test exists before the fix is implemented.

Blocks: #797

**Dependency (TDD workflow):** This TDD test issue blocks bug fix #797. This issue must be completed and merged first so the `@tdd_expected_fail` test exists before the fix is implemented. **Blocks:** #797
Author
Owner

PM Status — Day 37

Status: Not started. This TDD issue blocks bug #797 and has had no activity since the dependency note on Day 34.

@brent.edwards — Per the Week 6 schedule, TDD issues must be prioritized. Please provide an ETA or flag blockers by Day 38 EOD.


PM status — Day 37

## PM Status — Day 37 **Status**: Not started. This TDD issue blocks bug #797 and has had no activity since the dependency note on Day 34. @brent.edwards — Per the Week 6 schedule, TDD issues must be prioritized. Please provide an ETA or flag blockers by Day 38 EOD. --- *PM status — Day 37*
Member

Implementation Notes — TDD Tests for Bug #797

Root Cause Analysis

The bug originates in ActorRegistry.list_actors() (module cleveragents.actor.registry, method list_actors). This method unconditionally calls self.ensure_built_in_actors() before returning the actor list. ensure_built_in_actors() iterates all configured providers and calls self._actor_service.upsert_actor(...) for each one — a database write operation. Additionally, it may call self._actor_service.set_default_actor(...) if no default actor exists — another write.

This means the supposedly read-only agents actor list CLI command triggers database writes every time it runs, which can also trigger migration prompts on a fresh checkout.

Design Decisions

  1. Two Behave scenarios, two Robot test cases: One scenario asserts upsert_actor is never called (the primary write path), and a second asserts set_default_actor is never called (the secondary write path in ensure_built_in_actors()). Both are tagged with @tdd_expected_fail @tdd_bug @tdd_bug_797 at the feature level.

  2. Reused existing mock infrastructure: Tests use FakeProviderInfo and make_registry() from features/mocks/fake_provider.py, and patch cleveragents.cli.commands.actor._get_services to inject the mock service. This matches the established pattern in tdd_actor_list_validation_steps.py and actor_list_empty_steps.py.

  3. Step name disambiguation: All step definitions use the suffix for tdd-actor-list-no-db-update to avoid collisions with existing actor list test steps.

  4. Robot helper script pattern: The Robot helper (robot/helper_tdd_actor_list_no_db_update.py) follows the same dispatcher pattern as existing helpers (e.g., helper_tdd_plan_apply_yes_flag.py). Each subcommand exits 0 with a sentinel string when the bug is fixed, or exits 1 when the bug is present. The tdd_expected_fail_listener inverts the result.

  5. Originally planned a migration-output check scenario: A third scenario was initially designed to check for migration-related keywords in the CLI output. However, since the Behave tests use a MagicMock actor service (no real database), migration prompts never appear in the mocked context. The scenario was replaced with the set_default_actor check, which is a more reliable proof of the bug's write path.

Test Results

  • Behave: Both scenarios fail with AssertionError (proving the bug), then the @tdd_expected_fail inversion converts them to passes. 1 feature passed, 2 scenarios passed, 6 steps passed.
  • Robot: Both test cases fail (helper exits 1), then the tdd_expected_fail_listener inverts them to passes. 2 tests, 2 passed, 0 failed.
  • Full nox suite: lint , typecheck , unit_tests (pre-existing failures only), integration_tests (pre-existing failures only), coverage_report (98.22% > 97%).

Files Created

File Purpose
features/tdd_actor_list_no_db_update.feature Behave BDD scenarios with @tdd_expected_fail @tdd_bug @tdd_bug_797
features/steps/tdd_actor_list_no_db_update_steps.py Step definitions for the Behave scenarios
robot/tdd_actor_list_no_db_update.robot Robot Framework integration tests
robot/helper_tdd_actor_list_no_db_update.py Python helper script for Robot tests

Key Code Locations

  • Bug location: cleveragents.actor.registry.ActorRegistry.list_actorsensure_built_in_actors_actor_service.upsert_actor(...) and _actor_service.set_default_actor(...)
  • CLI entry point: cleveragents.cli.commands.actor.list_actorsregistry.list_actors()
  • Mock infrastructure: features.mocks.fake_provider.make_registry
## Implementation Notes — TDD Tests for Bug #797 ### Root Cause Analysis The bug originates in `ActorRegistry.list_actors()` (module `cleveragents.actor.registry`, method `list_actors`). This method unconditionally calls `self.ensure_built_in_actors()` before returning the actor list. `ensure_built_in_actors()` iterates all configured providers and calls `self._actor_service.upsert_actor(...)` for each one — a database **write** operation. Additionally, it may call `self._actor_service.set_default_actor(...)` if no default actor exists — another write. This means the supposedly read-only `agents actor list` CLI command triggers database writes every time it runs, which can also trigger migration prompts on a fresh checkout. ### Design Decisions 1. **Two Behave scenarios, two Robot test cases**: One scenario asserts `upsert_actor` is never called (the primary write path), and a second asserts `set_default_actor` is never called (the secondary write path in `ensure_built_in_actors()`). Both are tagged with `@tdd_expected_fail @tdd_bug @tdd_bug_797` at the feature level. 2. **Reused existing mock infrastructure**: Tests use `FakeProviderInfo` and `make_registry()` from `features/mocks/fake_provider.py`, and patch `cleveragents.cli.commands.actor._get_services` to inject the mock service. This matches the established pattern in `tdd_actor_list_validation_steps.py` and `actor_list_empty_steps.py`. 3. **Step name disambiguation**: All step definitions use the suffix `for tdd-actor-list-no-db-update` to avoid collisions with existing actor list test steps. 4. **Robot helper script pattern**: The Robot helper (`robot/helper_tdd_actor_list_no_db_update.py`) follows the same dispatcher pattern as existing helpers (e.g., `helper_tdd_plan_apply_yes_flag.py`). Each subcommand exits 0 with a sentinel string when the bug is fixed, or exits 1 when the bug is present. The `tdd_expected_fail_listener` inverts the result. 5. **Originally planned a migration-output check scenario**: A third scenario was initially designed to check for migration-related keywords in the CLI output. However, since the Behave tests use a `MagicMock` actor service (no real database), migration prompts never appear in the mocked context. The scenario was replaced with the `set_default_actor` check, which is a more reliable proof of the bug's write path. ### Test Results - **Behave**: Both scenarios fail with `AssertionError` (proving the bug), then the `@tdd_expected_fail` inversion converts them to passes. `1 feature passed, 2 scenarios passed, 6 steps passed`. - **Robot**: Both test cases fail (helper exits 1), then the `tdd_expected_fail_listener` inverts them to passes. `2 tests, 2 passed, 0 failed`. - **Full nox suite**: `lint` ✅, `typecheck` ✅, `unit_tests` ✅ (pre-existing failures only), `integration_tests` ✅ (pre-existing failures only), `coverage_report` ✅ (98.22% > 97%). ### Files Created | File | Purpose | |------|---------| | `features/tdd_actor_list_no_db_update.feature` | Behave BDD scenarios with `@tdd_expected_fail @tdd_bug @tdd_bug_797` | | `features/steps/tdd_actor_list_no_db_update_steps.py` | Step definitions for the Behave scenarios | | `robot/tdd_actor_list_no_db_update.robot` | Robot Framework integration tests | | `robot/helper_tdd_actor_list_no_db_update.py` | Python helper script for Robot tests | ### Key Code Locations - Bug location: `cleveragents.actor.registry.ActorRegistry.list_actors` → `ensure_built_in_actors` → `_actor_service.upsert_actor(...)` and `_actor_service.set_default_actor(...)` - CLI entry point: `cleveragents.cli.commands.actor.list_actors` → `registry.list_actors()` - Mock infrastructure: `features.mocks.fake_provider.make_registry`
Member

Verification & Rebase — Brent Edwards

Picked up the existing implementation on branch tdd/m3-actor-list-no-db-update (originally authored by @aditya), rebased onto latest master (commit 188a9edd), and ran the full quality gate suite to verify correctness.

Rebase

Branch was 4 commits behind master (two merge commits for LSP resource types and LSP functional runtime). Rebased cleanly with no conflicts.

Quality Gate Results (post-rebase)

Gate Result Notes
nox -e lint Pass All checks passed
nox -e typecheck Pass 0 errors, 1 pre-existing warning
nox -e unit_tests Pass 467 features, 12345 scenarios, all passed. Our 2 TDD scenarios pass via @tdd_expected_fail inversion
nox -e integration_tests Pass* Our 2 Robot tests pass via tdd_expected_fail_listener. 3 pre-existing failures (same on master)
nox -e e2e_tests Pass* Pre-existing failures only (fewer than master)
nox -e coverage_report 98% Above 97% threshold

* Pre-existing failures identical to or fewer than those on master — not caused by this branch.

TDD Behavior Verification

Behave scenarios (ran standalone):

  • upsert_actor assertion fails with: upsert_actor was called 1 time(s) during 'actor list' → proves bug #797 exists
  • set_default_actor assertion fails with: set_default_actor was called 1 time(s) during 'actor list' → proves bug #797 exists
  • @tdd_expected_fail inverts both to passes → CI green

Robot Framework tests (ran standalone):

  • Both check-no-upsert and check-no-set-default subcommands exit 1 (bug present)
  • tdd_expected_fail_listener inverts both to passes → CI green

Force-pushed

Rebased branch force-pushed to origin/tdd/m3-actor-list-no-db-update (commit 23df639e). PR #1148 updated automatically.

## Verification & Rebase — Brent Edwards Picked up the existing implementation on branch `tdd/m3-actor-list-no-db-update` (originally authored by @aditya), rebased onto latest `master` (commit `188a9edd`), and ran the full quality gate suite to verify correctness. ### Rebase Branch was 4 commits behind master (two merge commits for LSP resource types and LSP functional runtime). Rebased cleanly with no conflicts. ### Quality Gate Results (post-rebase) | Gate | Result | Notes | |------|--------|-------| | `nox -e lint` | ✅ Pass | All checks passed | | `nox -e typecheck` | ✅ Pass | 0 errors, 1 pre-existing warning | | `nox -e unit_tests` | ✅ Pass | 467 features, 12345 scenarios, all passed. Our 2 TDD scenarios pass via `@tdd_expected_fail` inversion | | `nox -e integration_tests` | ✅ Pass* | Our 2 Robot tests pass via `tdd_expected_fail_listener`. 3 pre-existing failures (same on master) | | `nox -e e2e_tests` | ✅ Pass* | Pre-existing failures only (fewer than master) | | `nox -e coverage_report` | ✅ 98% | Above 97% threshold | \* Pre-existing failures identical to or fewer than those on master — not caused by this branch. ### TDD Behavior Verification **Behave scenarios** (ran standalone): - `upsert_actor` assertion **fails** with: `upsert_actor was called 1 time(s) during 'actor list'` → proves bug #797 exists - `set_default_actor` assertion **fails** with: `set_default_actor was called 1 time(s) during 'actor list'` → proves bug #797 exists - `@tdd_expected_fail` inverts both to passes → CI green **Robot Framework tests** (ran standalone): - Both `check-no-upsert` and `check-no-set-default` subcommands exit 1 (bug present) - `tdd_expected_fail_listener` inverts both to passes → CI green ### Force-pushed Rebased branch force-pushed to `origin/tdd/m3-actor-list-no-db-update` (commit `23df639e`). PR #1148 updated automatically.
Sign in to join this conversation.
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
cleveragents/cleveragents-core#841
No description provided.