UAT: agents actor add does not enforce --update flag — re-adding an existing actor silently succeeds without --update #2609

Closed
opened 2026-04-03 19:36:26 +00:00 by freemo · 6 comments
Owner

Metadata

  • Branch: fix/actor-add-update-enforcement
  • Commit Message: fix(cli): enforce --update flag in agents actor add to prevent silent overwrites
  • Milestone: v3.6.0
  • Parent Epic: #392

Bug Report

Feature Area

Actor, Skill & Tool Abstraction — agents actor add CLI command

What Was Tested

Code analysis of src/cleveragents/cli/commands/actor.py — the add() command implementation.

Expected Behavior (from spec)

The specification (docs/specification.md, section agents actor add) states:

If a local actor with the same name already exists, the command fails unless the --update flag is provided, which replaces the existing actor.

When attempting to add an actor whose name is already registered (without --update), the command must fail with an error panel:

╭─ Error ─────────────────────────────────────────────────╮
│ Actor already exists: local/reviewer                    │
│ Registered: 2026-02-07 14:22                            │
│ Use --update to replace the existing actor definition.  │
╰─────────────────────────────────────────────────────────╯

✗ ERROR Actor already registered — use --update to replace

Actual Behavior

The add() command in src/cleveragents/cli/commands/actor.py calls registry.upsert_actor() (or service.upsert_actor()) unconditionally, regardless of whether --update was passed. The update_existing parameter is only used to:

  1. Change the panel title from "Actor added" to "Actor updated"
  2. Control whether to show the add-specific panels

There is no check for whether the actor already exists before calling upsert_actor(). Re-adding an existing actor without --update silently succeeds and overwrites the existing registration.

Code Location

src/cleveragents/cli/commands/actor.pyadd() function:

# The update_existing flag is declared but not used for existence check:
update_existing: Annotated[
    bool,
    typer.Option("--update", help="Update actor if it already exists"),
] = False,

# ...

# upsert_actor is called unconditionally — no existence check:
if registry:
    actor = registry.upsert_actor(
        name=name,
        config_blob=canonical_blob,
        ...
    )

Steps to Reproduce

  1. Add an actor: agents actor add --config ./actors/my-actor.yaml
  2. Add the same actor again without --update: agents actor add --config ./actors/my-actor.yaml
  3. Observe: Command succeeds and prints "Actor added" — no error
  4. Expected: Command should fail with "Actor already exists: local/my-actor"

Impact

  • Severity: High
  • Priority: High
  • The --update flag is documented as required for overwriting existing actors, but the enforcement is missing
  • Users can accidentally overwrite actor configurations without realizing it
  • The spec's error panel (with registration timestamp and hint) is never shown

Subtasks

  • 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
  • Error panel must include: actor name, registration timestamp, and hint to use --update
  • Add Behave tests for the duplicate-without-update failure case
  • Add Behave tests for the duplicate-with-update success case

Definition of Done

  • agents actor add fails with a clear error panel when the actor already exists and --update is not provided
  • agents actor add --update succeeds and overwrites the existing actor
  • Unit test coverage ≥ 97% maintained
  • PR merged and issue closed
  • All nox stages pass
  • Coverage >= 97%

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/actor-add-update-enforcement` - **Commit Message**: `fix(cli): enforce --update flag in agents actor add to prevent silent overwrites` - **Milestone**: v3.6.0 - **Parent Epic**: #392 ## Bug Report ### Feature Area Actor, Skill & Tool Abstraction — `agents actor add` CLI command ### What Was Tested Code analysis of `src/cleveragents/cli/commands/actor.py` — the `add()` command implementation. ### Expected Behavior (from spec) The specification (`docs/specification.md`, section `agents actor add`) states: > If a local actor with the same name already exists, the command fails unless the `--update` flag is provided, which replaces the existing actor. When attempting to add an actor whose name is already registered (without `--update`), the command must fail with an error panel: ``` ╭─ Error ─────────────────────────────────────────────────╮ │ Actor already exists: local/reviewer │ │ Registered: 2026-02-07 14:22 │ │ Use --update to replace the existing actor definition. │ ╰─────────────────────────────────────────────────────────╯ ✗ ERROR Actor already registered — use --update to replace ``` ### Actual Behavior The `add()` command in `src/cleveragents/cli/commands/actor.py` calls `registry.upsert_actor()` (or `service.upsert_actor()`) unconditionally, regardless of whether `--update` was passed. The `update_existing` parameter is only used to: 1. Change the panel title from "Actor added" to "Actor updated" 2. Control whether to show the add-specific panels There is **no check** for whether the actor already exists before calling `upsert_actor()`. Re-adding an existing actor without `--update` silently succeeds and overwrites the existing registration. ### Code Location `src/cleveragents/cli/commands/actor.py` — `add()` function: ```python # The update_existing flag is declared but not used for existence check: update_existing: Annotated[ bool, typer.Option("--update", help="Update actor if it already exists"), ] = False, # ... # upsert_actor is called unconditionally — no existence check: if registry: actor = registry.upsert_actor( name=name, config_blob=canonical_blob, ... ) ``` ### Steps to Reproduce 1. Add an actor: `agents actor add --config ./actors/my-actor.yaml` 2. Add the same actor again without `--update`: `agents actor add --config ./actors/my-actor.yaml` 3. Observe: Command succeeds and prints "Actor added" — no error 4. Expected: Command should fail with "Actor already exists: local/my-actor" ### Impact - **Severity**: High - **Priority**: High - The `--update` flag is documented as required for overwriting existing actors, but the enforcement is missing - Users can accidentally overwrite actor configurations without realizing it - The spec's error panel (with registration timestamp and hint) is never shown ## Subtasks - [ ] 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 - [ ] Error panel must include: actor name, registration timestamp, and hint to use `--update` - [ ] Add Behave tests for the duplicate-without-update failure case - [ ] Add Behave tests for the duplicate-with-update success case ## Definition of Done - [ ] `agents actor add` fails with a clear error panel when the actor already exists and `--update` is not provided - [ ] `agents actor add --update` succeeds and overwrites the existing actor - [ ] Unit test coverage ≥ 97% maintained - [ ] PR merged and issue closed - All nox stages pass - Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator
freemo added this to the v3.6.0 milestone 2026-04-03 19:38:04 +00:00
Author
Owner

Issue triaged by project owner:

  • State: Verified
  • MoSCoW: Should Have

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **MoSCoW**: Should Have --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner
Author
Owner

Issue triaged by project owner:

  • State: Verified
  • Priority: High — data safety issue; users can accidentally overwrite actor configurations without the required --update flag
  • Milestone: v3.6.0 (M7: Advanced Concepts & Deferred Features)
  • MoSCoW: Should Have — the spec explicitly requires --update enforcement for actor overwrites. Silent data loss is a significant UX and safety concern. This is a "SHOULD" level requirement.
  • Parent Epic: #392

Note: All development work is currently blocked by #2597 (CI quality gates broken on master). This issue will be ready for implementation once master is green.


Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **Priority**: High — data safety issue; users can accidentally overwrite actor configurations without the required `--update` flag - **Milestone**: v3.6.0 (M7: Advanced Concepts & Deferred Features) - **MoSCoW**: Should Have — the spec explicitly requires `--update` enforcement for actor overwrites. Silent data loss is a significant UX and safety concern. This is a "SHOULD" level requirement. - **Parent Epic**: #392 **Note:** All development work is currently blocked by #2597 (CI quality gates broken on master). This issue will be ready for implementation once master is green. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner
Author
Owner

Issue triaged by project owner:

  • State: Verified — Actor add command silently succeeds when re-adding without --update flag. Spec requires enforcement.
  • Existing Priority/High and MoSCoW/Should Have labels are appropriate.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified — Actor add command silently succeeds when re-adding without `--update` flag. Spec requires enforcement. - Existing Priority/High and MoSCoW/Should Have labels are appropriate. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner
Author
Owner

Starting implementation on branch fix/actor-add-update-enforcement.

Plan: Fix agents actor add to check for existing actor before calling upsert, and reject with error panel if --update flag is not provided.

Subtask wave analysis: All 5 subtasks analyzed — subtasks 1-3 (implementation) form Wave 1, subtasks 4-5 (tests) form Wave 2 (depend on implementation).

Difficulty assessment: Medium → starting at sonnet tier.


Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-worker

Starting implementation on branch `fix/actor-add-update-enforcement`. **Plan:** Fix `agents actor add` to check for existing actor before calling upsert, and reject with error panel if `--update` flag is not provided. **Subtask wave analysis:** All 5 subtasks analyzed — subtasks 1-3 (implementation) form Wave 1, subtasks 4-5 (tests) form Wave 2 (depend on implementation). Difficulty assessment: Medium → starting at sonnet tier. --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker
Author
Owner

All subtasks complete. Quality gates passed. Creating PR.

Implementation summary:

  • Added existence check in agents actor add before calling upsert_actor()
  • If actor exists and --update not provided: shows spec-required error panel with actor name, registration timestamp, and hint to use --update, exits with code 1
  • If actor exists and --update provided: proceeds with upsert (existing behavior)
  • If actor does not exist: proceeds with add (existing behavior)

Files changed:

  • src/cleveragents/cli/commands/actor.py — added existence check logic
  • features/actor_add_update_enforcement.feature — 4 Behave scenarios with @tdd_issue @tdd_issue_2609 tags
  • features/steps/actor_add_update_enforcement_steps.py — step definitions

Tests verified: All 3 behavioral paths confirmed working via direct Python test execution.


Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-worker

All subtasks complete. Quality gates passed. Creating PR. **Implementation summary:** - Added existence check in `agents actor add` before calling `upsert_actor()` - If actor exists and `--update` not provided: shows spec-required error panel with actor name, registration timestamp, and hint to use `--update`, exits with code 1 - If actor exists and `--update` provided: proceeds with upsert (existing behavior) - If actor does not exist: proceeds with add (existing behavior) **Files changed:** - `src/cleveragents/cli/commands/actor.py` — added existence check logic - `features/actor_add_update_enforcement.feature` — 4 Behave scenarios with `@tdd_issue @tdd_issue_2609` tags - `features/steps/actor_add_update_enforcement_steps.py` — step definitions **Tests verified:** All 3 behavioral paths confirmed working via direct Python test execution. --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker
Author
Owner

PR #3221 created on branch fix/actor-add-update-enforcement. PR review and merge handled by continuous review stream.

PR URL: #3221


Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-worker

PR #3221 created on branch `fix/actor-add-update-enforcement`. PR review and merge handled by continuous review stream. **PR URL:** https://git.cleverthis.com/cleveragents/cleveragents-core/pulls/3221 --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
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#2609
No description provided.