From 491781714fb1039a79f189b688588d8248e0a895 Mon Sep 17 00:00:00 2001 From: Jeffrey Phillips Freeman Date: Sun, 5 Apr 2026 07:49:49 +0000 Subject: [PATCH] fix(actors): enforce --update flag in agents actor add - reject re-adding existing actor without --update Before calling upsert_actor(), check if the actor already exists using registry.get_actor() or service.get_actor(). If the actor exists and update_existing is False, print the spec-required error panel and exit with error code 1. Error panel includes: - Actor name - Registration timestamp (formatted as YYYY-MM-DD HH:MM) - Hint to use --update flag Adds Behave tests for: - Duplicate-without-update failure case (exit code 1, error panel shown) - Duplicate-with-update success case (exit code 0, actor updated) - New actor without --update success case (exit code 0, actor added) ISSUES CLOSED: #2609 --- features/actor_add_update_enforcement.feature | 37 +++++ .../actor_add_update_enforcement_steps.py | 156 ++++++++++++++++++ src/cleveragents/cli/commands/actor.py | 22 +++ 3 files changed, 215 insertions(+) create mode 100644 features/actor_add_update_enforcement.feature create mode 100644 features/steps/actor_add_update_enforcement_steps.py diff --git a/features/actor_add_update_enforcement.feature b/features/actor_add_update_enforcement.feature new file mode 100644 index 000000000..fb414ae86 --- /dev/null +++ b/features/actor_add_update_enforcement.feature @@ -0,0 +1,37 @@ +# Regression tests for bug #2609: actor add must reject re-adding an existing +# actor unless --update is provided. +Feature: agents actor add enforces --update flag for existing actors + As a user of the CleverAgents CLI + I want `agents actor add` to fail with a clear error when re-adding an existing actor + So that I cannot accidentally overwrite actor configurations without explicit intent + + @tdd_issue @tdd_issue_2609 + Scenario: Re-adding an existing actor without --update fails with error panel + Given an actor add CLI runner where the actor already exists + When I run actor add without the --update flag + Then the actor-add-enforcement exit code should be 1 + And the actor-add-enforcement output should contain an error panel with "Actor already exists" + And the actor-add-enforcement output should contain "Use --update to replace the existing actor definition" + And the actor-add-enforcement output should contain the registration timestamp + + @tdd_issue @tdd_issue_2609 + Scenario: Re-adding an existing actor without --update shows error status line + Given an actor add CLI runner where the actor already exists + When I run actor add without the --update flag + Then the actor-add-enforcement exit code should be 1 + And the actor-add-enforcement output should contain "Actor already registered" + And the actor-add-enforcement output should contain "use --update to replace" + + @tdd_issue @tdd_issue_2609 + Scenario: Re-adding an existing actor with --update succeeds + Given an actor add CLI runner where the actor already exists + When I run actor add with the --update flag + Then the actor-add-enforcement exit code should be 0 + And the actor-add-enforcement output should contain "Actor updated" + + @tdd_issue @tdd_issue_2609 + Scenario: Adding a new actor without --update succeeds + Given an actor add CLI runner where the actor does not exist + When I run actor add without the --update flag + Then the actor-add-enforcement exit code should be 0 + And the actor-add-enforcement output should contain "Actor added" diff --git a/features/steps/actor_add_update_enforcement_steps.py b/features/steps/actor_add_update_enforcement_steps.py new file mode 100644 index 000000000..db6ee4801 --- /dev/null +++ b/features/steps/actor_add_update_enforcement_steps.py @@ -0,0 +1,156 @@ +"""Step definitions for actor add --update flag enforcement (issue #2609). + +Tests that `agents actor add` rejects re-adding an existing actor without +the --update flag, and succeeds when --update is provided. +""" + +from __future__ import annotations + +import json +import tempfile +from datetime import datetime +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock, patch + +from behave import given, then, when +from typer.testing import CliRunner + +from cleveragents.cli.commands.actor import app as actor_app +from cleveragents.core.exceptions import NotFoundError +from cleveragents.domain.models.core.actor import Actor + + +def _make_actor( + *, + name: str = "local/existing-actor", + provider: str = "openai", + model: str = "gpt-4", + config: dict[str, Any] | None = None, + updated_at: datetime | None = None, +) -> Actor: + blob = config or {} + return Actor( + id=1, + name=name, + provider=provider, + model=model, + config_blob=blob, + config_hash=Actor.compute_hash(blob), + unsafe=False, + is_built_in=False, + is_default=False, + updated_at=updated_at or datetime(2026, 2, 7, 14, 22, 0), + ) + + +def _register_cleanup(context: Any, path: Path) -> None: + context._cleanup_handlers.append(lambda: path.unlink(missing_ok=True)) + + +def _write_actor_config(name: str = "local/existing-actor") -> Path: + config_data = { + "name": name, + "provider": "openai", + "model": "gpt-4", + } + with tempfile.NamedTemporaryFile( + delete=False, suffix=".json", mode="w", encoding="utf-8" + ) as handle: + json.dump(config_data, handle) + handle.flush() + return Path(handle.name) + + +@given("an actor add CLI runner where the actor already exists") +def step_given_actor_exists(context: Any) -> None: + context.runner = CliRunner() + context.actor_name = "local/existing-actor" + context.actor_config_path = _write_actor_config(context.actor_name) + _register_cleanup(context, context.actor_config_path) + + # The existing actor that will be returned by get_actor + context.existing_actor = _make_actor( + name=context.actor_name, + updated_at=datetime(2026, 2, 7, 14, 22, 0), + ) + # The actor returned after a successful upsert (for --update case) + context.updated_actor = _make_actor( + name=context.actor_name, + updated_at=datetime(2026, 2, 7, 14, 22, 0), + ) + + +@given("an actor add CLI runner where the actor does not exist") +def step_given_actor_not_exists(context: Any) -> None: + context.runner = CliRunner() + context.actor_name = "local/new-actor" + context.actor_config_path = _write_actor_config(context.actor_name) + _register_cleanup(context, context.actor_config_path) + + context.new_actor = _make_actor(name=context.actor_name) + + +@when("I run actor add without the --update flag") +def step_when_add_without_update(context: Any) -> None: + with patch("cleveragents.cli.commands.actor._get_services") as mock_svc: + registry = MagicMock() + if hasattr(context, "existing_actor"): + # Actor exists — get_actor returns it, triggering the error path + registry.get_actor.return_value = context.existing_actor + else: + # Actor does not exist — get_actor raises NotFoundError + registry.get_actor.side_effect = NotFoundError( + resource_type="actor", resource_id=context.actor_name + ) + registry.upsert_actor.return_value = context.new_actor + mock_svc.return_value = (MagicMock(), registry) + context.result = context.runner.invoke( + actor_app, + ["add", "--config", str(context.actor_config_path)], + ) + + +@when("I run actor add with the --update flag") +def step_when_add_with_update(context: Any) -> None: + with patch("cleveragents.cli.commands.actor._get_services") as mock_svc: + registry = MagicMock() + # get_actor returns the existing actor (simulating actor already exists) + registry.get_actor.return_value = context.existing_actor + # upsert_actor returns the updated actor + registry.upsert_actor.return_value = context.updated_actor + mock_svc.return_value = (MagicMock(), registry) + context.result = context.runner.invoke( + actor_app, + ["add", "--config", str(context.actor_config_path), "--update"], + ) + + +@then("the actor-add-enforcement exit code should be {code:d}") +def step_then_exit_code(context: Any, code: int) -> None: + assert context.result.exit_code == code, ( + f"Expected exit code {code}, got {context.result.exit_code}.\n" + f"Output:\n{context.result.output}" + ) + + +@then('the actor-add-enforcement output should contain an error panel with "{text}"') +def step_then_output_contains_error_panel(context: Any, text: str) -> None: + output = context.result.output + assert text in output, f"Expected '{text}' in output:\n{output}" + assert "Error" in output, f"Expected 'Error' panel title in output:\n{output}" + + +@then('the actor-add-enforcement output should contain "{text}"') +def step_then_output_contains(context: Any, text: str) -> None: + output = context.result.output + assert text in output, f"Expected '{text}' in output:\n{output}" + + +@then("the actor-add-enforcement output should contain the registration timestamp") +def step_then_output_contains_timestamp(context: Any) -> None: + output = context.result.output + # The timestamp should be formatted as "YYYY-MM-DD HH:MM" + assert "2026-02-07 14:22" in output, ( + f"Expected registration timestamp '2026-02-07 14:22' in output:\n{output}" + ) diff --git a/src/cleveragents/cli/commands/actor.py b/src/cleveragents/cli/commands/actor.py index 8f4156644..868227058 100644 --- a/src/cleveragents/cli/commands/actor.py +++ b/src/cleveragents/cli/commands/actor.py @@ -493,6 +493,28 @@ def add( _print_role_warnings(canonical_blob) + # ── Enforce --update flag: reject re-adding an existing actor ───────────── + if not update_existing: + try: + existing = registry.get_actor(name) if registry else service.get_actor(name) + # Actor already exists and --update was not provided — reject with error + registered_ts = existing.updated_at.strftime("%Y-%m-%d %H:%M") + error_details = ( + f"Actor already exists: {name}\n" + f"Registered: {registered_ts}\n" + f"Use --update to replace the existing actor definition." + ) + console.print(Panel(error_details, title="Error", border_style="red")) + console.print( + "[red bold]✗ ERROR[/red bold] Actor already registered" + " \u2014 use --update to replace" + ) + raise typer.Exit(code=1) + except typer.Exit: + raise + except NotFoundError: + pass # Actor does not exist yet — proceed with add + try: if registry: actor = registry.upsert_actor( -- 2.52.0