fix(v3.7.0): resolve issue #1500 - actor add --update flag enforcement #1513

Merged
HAL9000 merged 7 commits from fix/1500-impl into master 2026-06-19 07:36:09 +00:00
6 changed files with 225 additions and 7 deletions
+4
View File
@@ -304,6 +304,10 @@ ensuring data is stored with proper parameter values.
- **Plan Rollback Command** (#8557): Implemented `agents plan rollback <plan-id> [<checkpoint-id>]` for checkpoint-based plan state restoration in Epic #8493. The command restores a plan's sandbox to the state captured at a given checkpoint, discarding all decisions made after that checkpoint. The checkpoint can be specified as an optional positional second argument or via the `--to-checkpoint` named option. Supports `--yes/-y` flag to skip confirmation prompts and `--format/-f` for output format selection (rich/plain/json/yaml). Included with comprehensive BDD test coverage (>= 97%) and spec-aligned output formatting showing rollback summary, changes reverted, impact analysis, and post-rollback state panels.
### Fixed
- **`agents actor add` enforces `--update` flag for existing actors** (#1500): Added regression tests to `features/actor_add_update_enforcement.feature` and step definitions in `features/steps/actor_add_update_enforcement_steps.py` verifying that re-adding an existing actor without `--update` fails, while re-adding with `--update` succeeds.
### Changed
- **Expanded ruff lint scope to cover `.opencode/`** (#10848): The `lint` nox session
+3
View File
1
@@ -56,6 +56,9 @@ Below are some of the specific details of various contributions.
* HAL 9000 has contributed the decision recording hook for the Strategize phase (issue #8522): captures every decision point with question, chosen option, alternatives, confidence, rationale, and full context snapshot for replay and correction.
* HAL 9000 has contributed the ContextStrategy protocol and StrategyRegistry plugin registration system (PR #10590 / issue #8616): implemented the pluggable context assembly strategy protocol with proper type-safe method signatures, created the central thread-safe StrategyRegistry supporting registration, lookup, entry-point discovery, and per-strategy configuration (timeout, fragments limits, workers, circuit breaker threshold). Six built-in strategies implemented and documented: simple-keyword, semantic-embedding, breadth-depth-navigator, arce, temporal-archaeology, and plan-decision-context. Full BDD test coverage including thread safety, boundary validation, and error handling tests. (Part of Epic #8505)
* HAL9000 has contributed automated implementation of ACMS context policy configuration loader and plan execution integration.
* HAL 9000 has contributed automated bug fixes, security improvements, and migration safety enhancements including the migration prompt safe-default fix (#7503).
* HAL 9000 has contributed the actor add --update flag enforcement tests and BDD coverage for issue #1500: added regression scenario tests verifying that re-adding an existing actor without `--update` fails, while re-adding with `--update` succeeds.
* This project was made possible thanks to considerable donation of time, money, and resources by CleverThis, Inc.
* HAL 9000 has contributed automated bug fixes, CLI output formatting improvements, and ongoing maintenance as part of the CleverAgents automation system.
* HAL 9000 has contributed the pr-review-pool-supervisor tracking prefix documentation fix (#7891): aligned all documentation references from the outdated `AUTO-REV-POOL` prefix to the correct `AUTO-REV-SUP` prefix used in production.
@@ -1,11 +1,11 @@
# Regression tests for bug #2609: actor add must reject re-adding an existing
# actor unless --update is provided.
# Regression tests for bug #1500/#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 @tdd_issue_4178
@tdd_issue @tdd_issue_1500 @tdd_issue_2609 @tdd_issue_4178
Outdated
Review

BLOCKING: Missing @tdd_issue_1500 tag. This PR closes issue #1500; per the TDD bug fix workflow in CONTRIBUTING.md, all regression scenarios must carry @tdd_issue_<N> for the issue being closed. The current tags are @tdd_issue @tdd_issue_2609 — add @tdd_issue_1500 to all 4 scenario tag lines.

Example fix:

  @tdd_issue @tdd_issue_1500 @tdd_issue_2609
  Scenario: Re-adding an existing actor without --update fails with error panel
**BLOCKING**: Missing `@tdd_issue_1500` tag. This PR closes issue #1500; per the TDD bug fix workflow in CONTRIBUTING.md, all regression scenarios must carry `@tdd_issue_<N>` for the issue being closed. The current tags are `@tdd_issue @tdd_issue_2609` — add `@tdd_issue_1500` to all 4 scenario tag lines. Example fix: ```gherkin @tdd_issue @tdd_issue_1500 @tdd_issue_2609 Scenario: Re-adding an existing actor without --update fails with error panel ```
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
@@ -14,7 +14,7 @@ Feature: agents actor add enforces --update flag for existing actors
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 @tdd_issue_4178
@tdd_issue @tdd_issue_1500 @tdd_issue_2609 @tdd_issue_4178
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
@@ -22,14 +22,14 @@ Feature: agents actor add enforces --update flag for existing actors
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 @tdd_issue_4178
@tdd_issue @tdd_issue_1500 @tdd_issue_2609 @tdd_issue_4178
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 @tdd_issue_4178
@tdd_issue @tdd_issue_1500 @tdd_issue_2609 @tdd_issue_4178
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
@@ -1,7 +1,11 @@
"""Step definitions for actor add --update flag enforcement (issue #2609).
"""Step definitions for actor add --update flag enforcement (issue #1500/#2609).
Tests that `agents actor add` rejects re-adding an existing actor without
the --update flag, and succeeds when --update is provided.
The ``add`` command requires a positional NAME argument followed by
``--config <FILE>``:
agents actor add <NAME> --config <FILE> [--update]
"""
from __future__ import annotations
@@ -105,6 +109,7 @@ def step_when_add_without_update(context: Any) -> None:
)
registry.upsert_actor.return_value = context.new_actor
mock_svc.return_value = (MagicMock(), registry)
# The add command requires a positional NAME argument before --config
context.result = context.runner.invoke(
actor_app,
["add", context.actor_name, "--config", str(context.actor_config_path)],
@@ -120,6 +125,7 @@ def step_when_add_with_update(context: Any) -> None:
# upsert_actor returns the updated actor
registry.upsert_actor.return_value = context.updated_actor
mock_svc.return_value = (MagicMock(), registry)
# The add command requires a positional NAME argument before --config
context.result = context.runner.invoke(
actor_app,
[
+36
View File
@@ -0,0 +1,36 @@
*** Settings ***
Documentation Integration tests for actor add --update flag enforcement (issue #1500)
Resource ${CURDIR}/common.resource
Suite Setup Setup Test Environment
Suite Teardown Cleanup Test Environment
*** Variables ***
${HELPER} ${CURDIR}/helper_actor_add_update_enforcement.py
*** Test Cases ***
Actor Add Existing Actor Without Update Flag Fails
[Documentation] Verify that actor add rejects re-adding an existing actor without --update
[Tags] tdd_issue tdd_issue_1500
${result}= Run Process ${PYTHON} ${HELPER} reject-existing-without-update cwd=${WORKSPACE}
Log ${result.stdout}
Log ${result.stderr}
Should Be Equal As Integers ${result.rc} 0
Should Contain ${result.stdout} actor-add-reject-existing-ok
Actor Add Existing Actor With Update Flag Succeeds
[Documentation] Verify that actor add with --update on an existing actor succeeds
[Tags] tdd_issue tdd_issue_1500
${result}= Run Process ${PYTHON} ${HELPER} accept-existing-with-update cwd=${WORKSPACE}
Log ${result.stdout}
Log ${result.stderr}
Should Be Equal As Integers ${result.rc} 0
Should Contain ${result.stdout} actor-add-accept-update-ok
Actor Add New Actor Without Update Flag Succeeds
[Documentation] Verify that actor add on a new actor succeeds without --update
[Tags] tdd_issue tdd_issue_1500
${result}= Run Process ${PYTHON} ${HELPER} accept-new-without-update cwd=${WORKSPACE}
Log ${result.stdout}
Log ${result.stderr}
Should Be Equal As Integers ${result.rc} 0
Should Contain ${result.stdout} actor-add-accept-new-ok
@@ -0,0 +1,169 @@
"""Helper script for actor add --update flag enforcement Robot tests (issue #1500).
Tests that ``agents actor add`` rejects re-adding an existing actor without
the --update flag, and succeeds when --update is provided or the actor is new.
"""
from __future__ import annotations
import json
import sys
import tempfile
from datetime import datetime
from pathlib import Path
from typing import Any
from unittest.mock import MagicMock, patch
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 _write_config(name: str) -> Path:
config_data: dict[str, Any] = {
"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)
def test_reject_existing_without_update() -> None:
"""Re-adding an existing actor without --update must exit 1 with an error panel."""
actor_name = "local/existing-actor"
config_path = _write_config(actor_name)
existing = _make_actor(name=actor_name, updated_at=datetime(2026, 2, 7, 14, 22, 0))
try:
runner = CliRunner()
with patch("cleveragents.cli.commands.actor._get_services") as mock_svc:
registry = MagicMock()
registry.get_actor.return_value = existing
mock_svc.return_value = (MagicMock(), registry)
result = runner.invoke(
actor_app,
["add", actor_name, "--config", str(config_path)],
)
finally:
config_path.unlink(missing_ok=True)
assert result.exit_code == 1, (
f"Expected exit code 1 but got {result.exit_code}.\nOutput:\n{result.output}"
)
assert "Actor already exists" in result.output, (
f"Expected 'Actor already exists' in output:\n{result.output}"
)
assert "Use --update to replace" in result.output, (
f"Expected 'Use --update to replace' in output:\n{result.output}"
)
print("actor-add-reject-existing-ok")
def test_accept_existing_with_update() -> None:
"""Re-adding an existing actor with --update must exit 0 and show
'Actor updated'."""
actor_name = "local/existing-actor"
config_path = _write_config(actor_name)
existing = _make_actor(name=actor_name, updated_at=datetime(2026, 2, 7, 14, 22, 0))
updated = _make_actor(name=actor_name, updated_at=datetime(2026, 2, 7, 14, 22, 0))
try:
runner = CliRunner()
with patch("cleveragents.cli.commands.actor._get_services") as mock_svc:
registry = MagicMock()
registry.get_actor.return_value = existing
registry.upsert_actor.return_value = updated
mock_svc.return_value = (MagicMock(), registry)
result = runner.invoke(
actor_app,
["add", actor_name, "--config", str(config_path), "--update"],
)
finally:
config_path.unlink(missing_ok=True)
assert result.exit_code == 0, (
f"Expected exit code 0 but got {result.exit_code}.\nOutput:\n{result.output}"
)
assert "Actor updated" in result.output, (
f"Expected 'Actor updated' in output:\n{result.output}"
)
print("actor-add-accept-update-ok")
def test_accept_new_without_update() -> None:
"""Adding a brand-new actor without --update must exit 0 and show 'Actor added'."""
actor_name = "local/new-actor"
config_path = _write_config(actor_name)
new_actor = _make_actor(name=actor_name)
try:
runner = CliRunner()
with patch("cleveragents.cli.commands.actor._get_services") as mock_svc:
registry = MagicMock()
registry.get_actor.side_effect = NotFoundError(
resource_type="actor", resource_id=actor_name
)
registry.upsert_actor.return_value = new_actor
mock_svc.return_value = (MagicMock(), registry)
result = runner.invoke(
actor_app,
["add", actor_name, "--config", str(config_path)],
)
finally:
config_path.unlink(missing_ok=True)
assert result.exit_code == 0, (
f"Expected exit code 0 but got {result.exit_code}.\nOutput:\n{result.output}"
)
assert "Actor added" in result.output, (
f"Expected 'Actor added' in output:\n{result.output}"
)
print("actor-add-accept-new-ok")
def main() -> None:
command = sys.argv[1] if len(sys.argv) > 1 else ""
dispatch = {
"reject-existing-without-update": test_reject_existing_without_update,
"accept-existing-with-update": test_accept_existing_with_update,
"accept-new-without-update": test_accept_new_without_update,
}
fn = dispatch.get(command)
if fn is None:
print(f"Unknown command: {command!r}", file=sys.stderr)
sys.exit(1)
fn()
if __name__ == "__main__":
main()