fix(actors): enforce --update flag in agents actor add - reject re-adding existing actor without --update
CI / lint (pull_request) Successful in 22s
CI / typecheck (pull_request) Successful in 57s
CI / security (pull_request) Successful in 54s
CI / quality (pull_request) Successful in 35s
CI / build (pull_request) Successful in 25s
CI / helm (pull_request) Successful in 23s
CI / unit_tests (pull_request) Failing after 6m58s
CI / docker (pull_request) Has been skipped
CI / e2e_tests (pull_request) Successful in 18m30s
CI / integration_tests (pull_request) Failing after 23m24s
CI / coverage (pull_request) Successful in 10m59s
CI / status-check (pull_request) Failing after 1s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 58m21s

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
This commit is contained in:
2026-04-05 07:49:49 +00:00
parent e2057f9117
commit 491781714f
3 changed files with 215 additions and 0 deletions
@@ -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"
@@ -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}"
)
+22
View File
@@ -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(