fix(cli): add NAME positional argument to agents actor add command per spec #3239

Merged
freemo merged 1 commit from fix/actor-add-name-positional-argument into master 2026-04-05 21:09:18 +00:00
Owner

Summary

The spec (docs/reference/actor_cli.md) defines the synopsis for agents actor add as:

agents actor add <NAME> --config <FILE> [--update] [--unsafe] [--set-default] [--option key=value] [--format FORMAT]

The implementation was missing the required <NAME> positional argument, silently reading the actor name from the config file's name field instead. This deviates from the spec and breaks the expected CLI UX.

Changes

  • src/cleveragents/cli/commands/actor.py: Add name as a required positional typer.Argument to the add command. Update docstring to match spec synopsis exactly. Remove the now-redundant config-file name validation (name comes from CLI arg, not config). The positional NAME takes precedence over any name field in the config file.

  • features/actor_add_name_positional.feature + features/steps/actor_add_name_positional_steps.py: New Behave BDD feature and step definitions for the NAME positional argument (TDD — written before the fix).

  • features/steps/actor_cli_steps.py, features/steps/actor_add_rich_output_steps.py, features/steps/actor_cli_yaml_steps.py: Updated all existing actor add step invocations to pass the NAME positional argument.

  • robot/helper_actor_add_rich_output.py, robot/helper_m2_e2e_verification.py: Updated Robot Framework helpers to pass NAME positional argument.

Before / After

Before (broken):

agents actor add --config ./actors/my-actor.yaml   # NAME read from config file

After (spec-compliant):

agents actor add local/my-actor --config ./actors/my-actor.yaml

Testing

  • New Behave BDD scenarios verify:
    1. NAME positional argument is accepted and passed to the registry
    2. NAME positional argument takes precedence over config file name field
    3. Missing NAME argument causes a CLI error (exit code 2)
  • All existing actor add tests updated to pass NAME positional argument
  • ruff check and ruff format pass on all modified files
  • pyright reports 0 errors on actor.py

Closes #2905


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

## Summary The spec (`docs/reference/actor_cli.md`) defines the synopsis for `agents actor add` as: ``` agents actor add <NAME> --config <FILE> [--update] [--unsafe] [--set-default] [--option key=value] [--format FORMAT] ``` The implementation was missing the required `<NAME>` positional argument, silently reading the actor name from the config file's `name` field instead. This deviates from the spec and breaks the expected CLI UX. ## Changes - **`src/cleveragents/cli/commands/actor.py`**: Add `name` as a required positional `typer.Argument` to the `add` command. Update docstring to match spec synopsis exactly. Remove the now-redundant config-file name validation (name comes from CLI arg, not config). The positional `NAME` takes precedence over any `name` field in the config file. - **`features/actor_add_name_positional.feature`** + **`features/steps/actor_add_name_positional_steps.py`**: New Behave BDD feature and step definitions for the `NAME` positional argument (TDD — written before the fix). - **`features/steps/actor_cli_steps.py`**, **`features/steps/actor_add_rich_output_steps.py`**, **`features/steps/actor_cli_yaml_steps.py`**: Updated all existing `actor add` step invocations to pass the `NAME` positional argument. - **`robot/helper_actor_add_rich_output.py`**, **`robot/helper_m2_e2e_verification.py`**: Updated Robot Framework helpers to pass `NAME` positional argument. ## Before / After **Before (broken):** ```bash agents actor add --config ./actors/my-actor.yaml # NAME read from config file ``` **After (spec-compliant):** ```bash agents actor add local/my-actor --config ./actors/my-actor.yaml ``` ## Testing - New Behave BDD scenarios verify: 1. `NAME` positional argument is accepted and passed to the registry 2. `NAME` positional argument takes precedence over config file `name` field 3. Missing `NAME` argument causes a CLI error (exit code 2) - All existing actor add tests updated to pass `NAME` positional argument - `ruff check` and `ruff format` pass on all modified files - `pyright` reports 0 errors on `actor.py` Closes #2905 --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker
fix(cli): add NAME positional argument to agents actor add command per spec
Some checks failed
CI / lint (pull_request) Successful in 28s
CI / typecheck (pull_request) Successful in 53s
CI / security (pull_request) Successful in 55s
CI / quality (pull_request) Successful in 34s
CI / build (pull_request) Successful in 17s
CI / helm (pull_request) Successful in 23s
CI / unit_tests (pull_request) Successful in 7m0s
CI / e2e_tests (pull_request) Failing after 16m33s
CI / integration_tests (pull_request) Successful in 23m25s
CI / docker (pull_request) Successful in 1m25s
CI / coverage (pull_request) Successful in 10m40s
CI / status-check (pull_request) Failing after 1s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 58m9s
2d07cd5ef8
The spec (docs/reference/actor_cli.md) defines the synopsis for
`agents actor add` as:

  agents actor add <NAME> --config <FILE> [--update] [--unsafe]
  [--set-default] [--option key=value] [--format FORMAT]

The implementation was missing the required <NAME> positional argument,
silently reading the actor name from the config file's `name` field
instead. This deviates from the spec and breaks the expected CLI UX.

Changes:
- Add `name` as a required positional Argument to the `add` command
- Update docstring to match spec synopsis exactly
- The positional NAME takes precedence over any `name` field in config
- Remove the now-redundant config-file name validation (name comes from CLI)
- Add Behave BDD feature + steps for the NAME positional argument (TDD)
- Update all existing Behave step invocations to pass NAME positional arg
- Update Robot Framework helpers to pass NAME positional arg

ISSUES CLOSED: #2905
Author
Owner

🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-3239-1775372600]


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-continuous-pr-reviewer

🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-3239-1775372600] --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-continuous-pr-reviewer
freemo left a comment

Independent Code Review

Summary

This PR correctly fixes a spec deviation where agents actor add was missing the required <NAME> positional argument. The spec (docs/reference/actor_cli.md) clearly defines the synopsis as:

agents actor add <NAME> --config <FILE> [--update] [--unsafe] [--set-default] [--option key=value] [--format FORMAT]

The implementation was silently reading the actor name from the config file's name field instead. This PR brings the CLI into alignment with the spec.

What Was Reviewed

Specification Alignment

  • The name parameter is now a required typer.Argument with metavar="NAME" — matches spec synopsis exactly
  • The docstring and examples are updated to match the spec
  • The YAML config example in the spec does NOT include a name field, confirming name should come from CLI
  • The old validation requiring name in the config file is correctly removed

Code Quality

  • Proper Annotated[str, typer.Argument(...)] typing — no # type: ignore
  • Clean removal of the redundant config-file name validation (6 lines removed)
  • name is correctly passed through to _canonicalize_actor_config(), registry.upsert_actor(), and service.upsert_actor()
  • No unnecessary complexity introduced

Test Quality

  • 3 new Behave BDD scenarios covering: acceptance, precedence over config name, and missing argument error
  • Step definitions properly mock _get_services and verify upsert_actor is called with the correct name
  • All existing test files updated consistently (actor_cli_steps.py, actor_add_rich_output_steps.py, actor_cli_yaml_steps.py)
  • Robot Framework helpers updated (helper_actor_add_rich_output.py, helper_m2_e2e_verification.py)

Commit Format

  • Conventional Changelog: fix(cli): add NAME positional argument to agents actor add command per spec
  • ISSUES CLOSED: #2905 footer present
  • Single commit — clean history

Security

  • No secrets or credentials in code
  • No new input validation concerns (typer handles argument parsing)

Minor Note

The PR is missing a milestone assignment (issue #2905 has milestone v3.7.0). This is a metadata gap but does not block the review.

Verdict

Code is correct, spec-aligned, well-tested, and ready to merge. Scheduling merge for when all CI checks pass.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## Independent Code Review ✅ ### Summary This PR correctly fixes a spec deviation where `agents actor add` was missing the required `<NAME>` positional argument. The spec (`docs/reference/actor_cli.md`) clearly defines the synopsis as: ``` agents actor add <NAME> --config <FILE> [--update] [--unsafe] [--set-default] [--option key=value] [--format FORMAT] ``` The implementation was silently reading the actor name from the config file's `name` field instead. This PR brings the CLI into alignment with the spec. ### What Was Reviewed **Specification Alignment** ✅ - The `name` parameter is now a required `typer.Argument` with `metavar="NAME"` — matches spec synopsis exactly - The docstring and examples are updated to match the spec - The YAML config example in the spec does NOT include a `name` field, confirming name should come from CLI - The old validation requiring `name` in the config file is correctly removed **Code Quality** ✅ - Proper `Annotated[str, typer.Argument(...)]` typing — no `# type: ignore` - Clean removal of the redundant config-file name validation (6 lines removed) - `name` is correctly passed through to `_canonicalize_actor_config()`, `registry.upsert_actor()`, and `service.upsert_actor()` - No unnecessary complexity introduced **Test Quality** ✅ - 3 new Behave BDD scenarios covering: acceptance, precedence over config name, and missing argument error - Step definitions properly mock `_get_services` and verify `upsert_actor` is called with the correct name - All existing test files updated consistently (actor_cli_steps.py, actor_add_rich_output_steps.py, actor_cli_yaml_steps.py) - Robot Framework helpers updated (helper_actor_add_rich_output.py, helper_m2_e2e_verification.py) **Commit Format** ✅ - Conventional Changelog: `fix(cli): add NAME positional argument to agents actor add command per spec` - `ISSUES CLOSED: #2905` footer present - Single commit — clean history **Security** ✅ - No secrets or credentials in code - No new input validation concerns (typer handles argument parsing) ### Minor Note The PR is missing a milestone assignment (issue #2905 has milestone v3.7.0). This is a metadata gap but does not block the review. ### Verdict Code is correct, spec-aligned, well-tested, and ready to merge. Scheduling merge for when all CI checks pass. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer
freemo scheduled this pull request to auto merge when all checks succeed 2026-04-05 09:08:08 +00:00
freemo merged commit eb4ccc819c into master 2026-04-05 21:09:14 +00:00
Sign in to join this conversation.
No reviewers
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!3239
No description provided.