diff --git a/features/steps/tdd_actor_list_no_db_update_steps.py b/features/steps/tdd_actor_list_no_db_update_steps.py new file mode 100644 index 000000000..e36af6d4f --- /dev/null +++ b/features/steps/tdd_actor_list_no_db_update_steps.py @@ -0,0 +1,96 @@ +"""Step definitions for TDD Bug #797 — actor list should not cause a database update. + +These steps verify that ``agents actor list`` does **not** trigger +``upsert_actor()`` or ``set_default_actor()`` — both are database write +operations. While the bug is present the assertions fail; the +``@tdd_expected_fail`` tag on the feature inverts the result so CI passes. +""" + +from __future__ import annotations + +from unittest.mock import patch + +from behave import given, then, when # type: ignore[import-untyped] +from behave.runner import Context # type: ignore[import-untyped] +from typer.testing import CliRunner + +from cleveragents.cli.commands.actor import app as actor_app +from features.mocks.fake_provider import FakeProviderInfo, make_registry + +_PATCH_TARGET: str = "cleveragents.cli.commands.actor._get_services" + +_DEFAULT_PROVIDERS: list[FakeProviderInfo] = [ + FakeProviderInfo( + name="FakeProvider", + default_model="fake-org/fake-model-v1", + ), +] + + +# --------------------------------------------------------------------------- +# Given +# --------------------------------------------------------------------------- + + +@given( + "a mock actor service with a fake provider registry for tdd-actor-list-no-db-update" +) +def step_given_mock_service(context: Context) -> None: + """Set up a ``MagicMock`` actor service wired to a fake provider registry.""" + mock_service, registry = make_registry(_DEFAULT_PROVIDERS) + context.mock_service = mock_service + context.registry = registry + context.runner = CliRunner() + + +# --------------------------------------------------------------------------- +# When +# --------------------------------------------------------------------------- + + +@when("the actor list command is invoked for tdd-actor-list-no-db-update") +def step_when_invoke_actor_list(context: Context) -> None: + """Invoke ``actor list`` through the Typer test runner.""" + with patch( + _PATCH_TARGET, + return_value=(context.mock_service, context.registry), + ): + context.result = context.runner.invoke(actor_app, ["list"]) + + +# --------------------------------------------------------------------------- +# Then +# --------------------------------------------------------------------------- + + +@then("upsert_actor should not have been called for tdd-actor-list-no-db-update") +def step_then_no_upsert(context: Context) -> None: + """Assert that ``upsert_actor`` was never called. + + Bug #797: ``ActorRegistry.list_actors()`` calls + ``ensure_built_in_actors()`` which invokes ``upsert_actor()`` for + every configured provider — a database write on a read-only command. + """ + call_count: int = context.mock_service.upsert_actor.call_count + assert call_count == 0, ( + f"upsert_actor was called {call_count} time(s) during 'actor list'. " + f"A read-only list command must not trigger database writes " + f"(bug #797)." + ) + + +@then("set_default_actor should not have been called for tdd-actor-list-no-db-update") +def step_then_no_set_default(context: Context) -> None: + """Assert that ``set_default_actor`` was never called. + + Bug #797: ``ActorRegistry.list_actors()`` calls + ``ensure_built_in_actors()`` which may invoke + ``set_default_actor()`` — another database write triggered by a + read-only list command. + """ + call_count: int = context.mock_service.set_default_actor.call_count + assert call_count == 0, ( + f"set_default_actor was called {call_count} time(s) during " + f"'actor list'. A read-only list command must not trigger " + f"database writes (bug #797)." + ) diff --git a/features/tdd_actor_list_no_db_update.feature b/features/tdd_actor_list_no_db_update.feature new file mode 100644 index 000000000..f11b8114f --- /dev/null +++ b/features/tdd_actor_list_no_db_update.feature @@ -0,0 +1,22 @@ +@tdd_expected_fail @tdd_bug @tdd_bug_797 +Feature: TDD Bug #797 — actor list should not cause a database update + + Bug #797 reports that running ``agents actor list`` on a fresh checkout + triggers pending-migration prompts and database writes. The ``list`` + command is read-only from the user's perspective, so it must not call + ``upsert_actor()`` or any other write operation. + + These scenarios prove the bug exists by asserting that no database + writes occur during ``actor list``. While the bug is unfixed the + assertions **fail** (proving the defect) and the ``@tdd_expected_fail`` + tag inverts the result so CI stays green. + + Scenario: Actor list does not call upsert_actor for tdd-actor-list-no-db-update + Given a mock actor service with a fake provider registry for tdd-actor-list-no-db-update + When the actor list command is invoked for tdd-actor-list-no-db-update + Then upsert_actor should not have been called for tdd-actor-list-no-db-update + + Scenario: Actor list does not call set_default_actor for tdd-actor-list-no-db-update + Given a mock actor service with a fake provider registry for tdd-actor-list-no-db-update + When the actor list command is invoked for tdd-actor-list-no-db-update + Then set_default_actor should not have been called for tdd-actor-list-no-db-update diff --git a/robot/helper_tdd_actor_list_no_db_update.py b/robot/helper_tdd_actor_list_no_db_update.py new file mode 100644 index 000000000..588802677 --- /dev/null +++ b/robot/helper_tdd_actor_list_no_db_update.py @@ -0,0 +1,124 @@ +"""Helper script for tdd_actor_list_no_db_update.robot smoke tests. + +Each subcommand exercises the real ``ActorRegistry.list_actors()`` code +path to reproduce bug #797. The ``actor list`` command is invoked via +:class:`typer.testing.CliRunner` with a mock actor service and fake +provider registry, and the helper checks whether ``upsert_actor()`` or +``set_default_actor()`` was called (database writes). + +The helper reports the **real** outcome: it exits 0 and prints the +sentinel when the bug is fixed (no writes), and exits 1 when the bug +is still present (writes detected). The ``tdd_expected_fail_listener`` +on the Robot side handles pass/fail inversion while the bug remains open. +""" + +from __future__ import annotations + +import sys +from collections.abc import Callable +from pathlib import Path +from typing import NoReturn +from unittest.mock import patch + +# Ensure local source tree is importable. +_ROOT: Path = Path(__file__).resolve().parents[1] +_SRC: str = str(_ROOT / "src") +_FEATURES: str = str(_ROOT / "features") +for _p in (_SRC, _FEATURES): + if _p not in sys.path: + sys.path.insert(0, _p) + +from mocks.fake_provider import FakeProviderInfo, make_registry # noqa: E402 +from typer.testing import CliRunner # noqa: E402 + +from cleveragents.cli.commands.actor import app as actor_app # noqa: E402 + +runner: CliRunner = CliRunner() + +_PATCH_TARGET: str = "cleveragents.cli.commands.actor._get_services" + +_DEFAULT_PROVIDERS: list[FakeProviderInfo] = [ + FakeProviderInfo( + name="FakeProvider", + default_model="fake-org/fake-model-v1", + ), +] + + +def _fail(message: str) -> NoReturn: + """Print an error message to stderr and exit with code 1.""" + print(message, file=sys.stderr) + sys.exit(1) + + +# --------------------------------------------------------------------------- +# Subcommands +# --------------------------------------------------------------------------- + + +def check_no_upsert() -> None: + """Verify that ``actor list`` does not call ``upsert_actor``. + + Exits 0 with sentinel when ``upsert_actor`` was NOT called (bug + fixed). Exits 1 when ``upsert_actor`` WAS called (bug still + present). + """ + mock_service, registry = make_registry(_DEFAULT_PROVIDERS) + with patch( + _PATCH_TARGET, + return_value=(mock_service, registry), + ): + runner.invoke(actor_app, ["list"]) + + call_count: int = mock_service.upsert_actor.call_count + if call_count > 0: + _fail( + f"upsert_actor was called {call_count} time(s) during " + f"'actor list'. A read-only list command must not trigger " + f"database writes (bug #797)." + ) + print("tdd-actor-list-no-db-update-no-upsert-ok") + + +def check_no_set_default() -> None: + """Verify that ``actor list`` does not call ``set_default_actor``. + + Exits 0 with sentinel when ``set_default_actor`` was NOT called + (bug fixed). Exits 1 when ``set_default_actor`` WAS called (bug + still present). + """ + mock_service, registry = make_registry(_DEFAULT_PROVIDERS) + with patch( + _PATCH_TARGET, + return_value=(mock_service, registry), + ): + runner.invoke(actor_app, ["list"]) + + call_count: int = mock_service.set_default_actor.call_count + if call_count > 0: + _fail( + f"set_default_actor was called {call_count} time(s) during " + f"'actor list'. A read-only list command must not trigger " + f"database writes (bug #797)." + ) + print("tdd-actor-list-no-db-update-no-set-default-ok") + + +# --------------------------------------------------------------------------- +# Dispatcher +# --------------------------------------------------------------------------- + +_COMMANDS: dict[str, Callable[[], None]] = { + "check-no-upsert": check_no_upsert, + "check-no-set-default": check_no_set_default, +} + +if __name__ == "__main__": + if len(sys.argv) < 2 or sys.argv[1] not in _COMMANDS: + print( + f"Usage: {sys.argv[0]} <{'|'.join(_COMMANDS)}>", + file=sys.stderr, + ) + sys.exit(1) + cmd: Callable[[], None] = _COMMANDS[sys.argv[1]] + cmd() diff --git a/robot/tdd_actor_list_no_db_update.robot b/robot/tdd_actor_list_no_db_update.robot new file mode 100644 index 000000000..3c8a1e17c --- /dev/null +++ b/robot/tdd_actor_list_no_db_update.robot @@ -0,0 +1,40 @@ +*** Settings *** +Documentation TDD Bug #797 -- actor list should not cause a database update +... Integration smoke tests verifying that the ``actor list`` +... command does not trigger ``upsert_actor()`` or +... ``set_default_actor()`` calls. While the bug is present +... the helper exits 1 and the ``tdd_expected_fail_listener`` +... inverts the failure to a pass. +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER} ${CURDIR}/helper_tdd_actor_list_no_db_update.py + +*** Test Cases *** +TDD Actor List Does Not Call Upsert Actor + [Documentation] Verify that ``actor list`` does not call + ... ``upsert_actor()`` on the actor service. + ... Bug #797: ``list_actors()`` calls + ... ``ensure_built_in_actors()`` which writes to the DB. + [Tags] tdd_expected_fail tdd_bug tdd_bug_797 + ${result}= Run Process ${PYTHON} ${HELPER} check-no-upsert + ... cwd=${WORKSPACE} timeout=30s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-actor-list-no-db-update-no-upsert-ok + +TDD Actor List Does Not Call Set Default Actor + [Documentation] Verify that ``actor list`` does not call + ... ``set_default_actor()`` on the actor service. + ... Bug #797: ``ensure_built_in_actors()`` may also + ... call ``set_default_actor()`` — another DB write. + [Tags] tdd_expected_fail tdd_bug tdd_bug_797 + ${result}= Run Process ${PYTHON} ${HELPER} check-no-set-default + ... cwd=${WORKSPACE} timeout=30s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} tdd-actor-list-no-db-update-no-set-default-ok