fix(cli): make actor NAME argument optional, derive from YAML config
CI / lint (push) Successful in 1m6s
CI / helm (push) Successful in 42s
CI / build (push) Successful in 1m12s
CI / quality (push) Successful in 1m28s
CI / typecheck (push) Successful in 1m30s
CI / security (push) Successful in 2m23s
CI / push-validation (push) Successful in 21s
CI / e2e_tests (push) Successful in 4m5s
CI / integration_tests (push) Successful in 4m18s
CI / unit_tests (push) Successful in 4m56s
CI / docker (push) Successful in 1m29s
CI / benchmark-regression (push) Has been skipped
CI / coverage (push) Successful in 10m33s
CI / status-check (push) Successful in 4s
CI / benchmark-publish (push) Successful in 1h17m53s
CI / benchmark-publish (pull_request) Has been skipped
CI / coverage (pull_request) Successful in 10m54s
CI / push-validation (pull_request) Successful in 40s
CI / build (pull_request) Successful in 1m2s
CI / helm (pull_request) Successful in 44s
CI / lint (pull_request) Successful in 1m15s
CI / quality (pull_request) Successful in 1m32s
CI / typecheck (pull_request) Successful in 1m40s
CI / security (pull_request) Successful in 1m41s
CI / integration_tests (pull_request) Successful in 4m55s
CI / e2e_tests (pull_request) Successful in 4m54s
CI / unit_tests (pull_request) Successful in 6m29s
CI / docker (pull_request) Successful in 1m36s
CI / status-check (pull_request) Successful in 3s
CI / benchmark-regression (pull_request) Failing after 14m44s
CI / lint (push) Successful in 1m6s
CI / helm (push) Successful in 42s
CI / build (push) Successful in 1m12s
CI / quality (push) Successful in 1m28s
CI / typecheck (push) Successful in 1m30s
CI / security (push) Successful in 2m23s
CI / push-validation (push) Successful in 21s
CI / e2e_tests (push) Successful in 4m5s
CI / integration_tests (push) Successful in 4m18s
CI / unit_tests (push) Successful in 4m56s
CI / docker (push) Successful in 1m29s
CI / benchmark-regression (push) Has been skipped
CI / coverage (push) Successful in 10m33s
CI / status-check (push) Successful in 4s
CI / benchmark-publish (push) Successful in 1h17m53s
CI / benchmark-publish (pull_request) Has been skipped
CI / coverage (pull_request) Successful in 10m54s
CI / push-validation (pull_request) Successful in 40s
CI / build (pull_request) Successful in 1m2s
CI / helm (pull_request) Successful in 44s
CI / lint (pull_request) Successful in 1m15s
CI / quality (pull_request) Successful in 1m32s
CI / typecheck (pull_request) Successful in 1m40s
CI / security (pull_request) Successful in 1m41s
CI / integration_tests (pull_request) Successful in 4m55s
CI / e2e_tests (pull_request) Successful in 4m54s
CI / unit_tests (pull_request) Successful in 6m29s
CI / docker (pull_request) Successful in 1m36s
CI / status-check (pull_request) Successful in 3s
CI / benchmark-regression (pull_request) Failing after 14m44s
Implement NAME argument as optional with derivation from config file when not provided.
Changes:
- actor.py: NAME argument changed from required 'str' to optional 'str | None = None'
- Added help text explaining NAME can be omitted and derived from config
- Added logic to derive name from config_blob.get('name') when name arg is None
- Raises BadParameter if neither name argument nor config 'name' field is provided
- Updated docstring signature to reflect optional NAME: 'agents actor add [--config|-c <FILE>] [<NAME>]'
- Updated examples to show config-only usage
ISSUES CLOSED: #4186
This commit was merged in pull request #10974.
This commit is contained in:
@@ -6,6 +6,19 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
- **Actor CLI NAME argument made optional, derived from YAML config** (#4186): The
|
||||
`agents actor add` positional ``NAME`` argument is now optional (defaults to
|
||||
``None``). When omitted, the actor name is derived from the ``name`` field in
|
||||
the config file. Raises ``BadParameter`` if neither the argument nor the config
|
||||
``name`` field is provided. Updated docstring signature to
|
||||
``agents actor add [--config|-c <FILE>] [<NAME>]`` and added config-only usage
|
||||
examples. Added Behave scenario for the ``BadParameter`` error path
|
||||
(``actor add without NAME and without config name field raises BadParameter``)
|
||||
in ``features/actor_add_name_positional.feature`` with corresponding step
|
||||
definition. Updated step definitions in
|
||||
``features/steps/actor_add_update_enforcement_steps.py`` and
|
||||
``features/steps/actor_add_name_positional_steps.py`` to pass ``context.actor_name``
|
||||
as a positional argument for compatibility.
|
||||
|
||||
- **Improved parallel test suite isolation** (#4186): Replaced deprecated
|
||||
``tempfile.mktemp`` with ``tempfile.mkstemp`` in ``features/environment.py``
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
Feature: agents actor add NAME positional argument
|
||||
As a user of the CleverAgents CLI
|
||||
I want to pass the actor name as a positional argument to `agents actor add`
|
||||
So that the CLI matches the spec synopsis: agents actor add <NAME> --config <FILE>
|
||||
So that the CLI matches the spec synopsis: agents actor add --config <FILE> [<NAME>]
|
||||
|
||||
@tdd_issue @tdd_issue_4230 @tdd_expected_fail
|
||||
Scenario: actor add accepts NAME as positional argument
|
||||
@@ -17,8 +17,16 @@ Feature: agents actor add NAME positional argument
|
||||
When I run actor add with NAME positional argument overriding config name
|
||||
Then the actor add should use the positional NAME not the config name
|
||||
|
||||
Scenario: actor add without NAME positional argument fails
|
||||
@tdd_issue @tdd_issue_4186
|
||||
Scenario: actor add without NAME positional argument uses config name
|
||||
Given an actor CLI runner
|
||||
And I have an actor JSON config file with name "local/config-derived-actor"
|
||||
When I run actor add with config but no NAME positional argument
|
||||
Then the actor add should succeed using the config name
|
||||
|
||||
@tdd_issue @tdd_issue_4186
|
||||
Scenario: actor add without NAME and without config name field raises BadParameter
|
||||
Given an actor CLI runner
|
||||
And I have an actor JSON config file without a name field
|
||||
When I run actor add with config but no NAME positional argument
|
||||
Then the actor command should fail with missing argument error
|
||||
Then the actor add should fail with a BadParameter error about missing actor name
|
||||
|
||||
@@ -11,6 +11,7 @@ from unittest.mock import MagicMock, patch
|
||||
from behave import given, then, when
|
||||
|
||||
from cleveragents.cli.commands.actor import app as actor_app
|
||||
from cleveragents.core.exceptions import NotFoundError
|
||||
from cleveragents.domain.models.core.actor import Actor
|
||||
|
||||
|
||||
@@ -75,6 +76,22 @@ def step_impl(context: Any) -> None:
|
||||
_register_cleanup(context, context.actor_config_path)
|
||||
|
||||
|
||||
@given("I have an actor JSON config file with name {name}")
|
||||
def step_impl(context: Any, name: str) -> None:
|
||||
context.actor_config_data = {
|
||||
"name": name,
|
||||
"provider": "openai",
|
||||
"model": "gpt-4o-mini",
|
||||
}
|
||||
with tempfile.NamedTemporaryFile(
|
||||
delete=False, suffix=".json", mode="w", encoding="utf-8"
|
||||
) as handle:
|
||||
json.dump(context.actor_config_data, handle)
|
||||
handle.flush()
|
||||
context.actor_config_path = Path(handle.name)
|
||||
_register_cleanup(context, context.actor_config_path)
|
||||
|
||||
|
||||
@when("I run actor add with NAME positional argument and config")
|
||||
def step_impl(context: Any) -> None:
|
||||
context.positional_name = "local/my-actor"
|
||||
@@ -117,8 +134,14 @@ def step_impl(context: Any) -> None:
|
||||
|
||||
@when("I run actor add with config but no NAME positional argument")
|
||||
def step_impl(context: Any) -> None:
|
||||
config_name = context.actor_config_data.get("name", "local/default-actor")
|
||||
mock_actor = _make_actor(name=config_name)
|
||||
with patch("cleveragents.cli.commands.actor._get_services") as mock_svc:
|
||||
registry = MagicMock()
|
||||
registry.get_actor.side_effect = NotFoundError(
|
||||
f"Actor not found: {config_name}"
|
||||
)
|
||||
registry.upsert_actor.return_value = mock_actor
|
||||
mock_svc.return_value = (MagicMock(), registry)
|
||||
context.result = context.runner.invoke(
|
||||
actor_app,
|
||||
@@ -128,6 +151,7 @@ def step_impl(context: Any) -> None:
|
||||
str(context.actor_config_path),
|
||||
],
|
||||
)
|
||||
context.mock_registry = registry
|
||||
|
||||
|
||||
@then("the actor add should succeed with the positional name")
|
||||
@@ -175,3 +199,34 @@ def step_impl(context: Any) -> None:
|
||||
f"Expected non-zero exit_code, got {context.result.exit_code}.\n"
|
||||
f"Output:\n{context.result.output}"
|
||||
)
|
||||
|
||||
|
||||
@then("the actor add should succeed using the config name")
|
||||
def step_impl(context: Any) -> None:
|
||||
assert context.result.exit_code == 0, (
|
||||
f"Expected exit_code=0, got {context.result.exit_code}.\n"
|
||||
f"Output:\n{context.result.output}"
|
||||
)
|
||||
assert context.mock_registry.upsert_actor.called, (
|
||||
"Expected upsert_actor to be called on the registry"
|
||||
)
|
||||
call_kwargs = context.mock_registry.upsert_actor.call_args
|
||||
actual_name = call_kwargs.kwargs.get("name") or (
|
||||
call_kwargs.args[0] if call_kwargs.args else None
|
||||
)
|
||||
expected_name = context.actor_config_data.get("name")
|
||||
assert actual_name == expected_name, (
|
||||
f"Expected upsert_actor called with name={expected_name!r} (from config), "
|
||||
f"got name={actual_name!r}"
|
||||
)
|
||||
|
||||
|
||||
@then("the actor add should fail with a BadParameter error about missing actor name")
|
||||
def step_impl(context: Any) -> None:
|
||||
assert context.result.exit_code != 0, (
|
||||
f"Expected non-zero exit_code, got {context.result.exit_code}.\n"
|
||||
f"Output:\n{context.result.output}"
|
||||
)
|
||||
assert "Actor name is required" in context.result.output, (
|
||||
f"Expected 'Actor name is required' in output:\n{context.result.output}"
|
||||
)
|
||||
|
||||
@@ -534,12 +534,13 @@ def _print_role_warnings(config_blob: dict[str, Any]) -> None:
|
||||
@app.command()
|
||||
def add(
|
||||
name: Annotated[
|
||||
str,
|
||||
str | None,
|
||||
typer.Argument(
|
||||
help="Namespaced actor name (e.g. local/my-actor)",
|
||||
help="Namespaced actor name (e.g. local/my-actor). "
|
||||
"If omitted, the name is derived from the config file.",
|
||||
metavar="NAME",
|
||||
),
|
||||
],
|
||||
] = None,
|
||||
config: Annotated[
|
||||
Path | None,
|
||||
typer.Option("--config", "-c", help="Path to JSON/YAML actor config"),
|
||||
@@ -569,19 +570,19 @@ def add(
|
||||
) -> None:
|
||||
"""Add a new actor configuration.
|
||||
|
||||
The actor name is provided as a required positional argument. The YAML/JSON
|
||||
configuration file specified with ``--config`` supplies the actor settings.
|
||||
The positional ``NAME`` takes precedence over any ``name`` field in the
|
||||
config file.
|
||||
The YAML/JSON configuration file specified with ``--config`` supplies the
|
||||
actor settings. The actor's registered name is taken from the config file's
|
||||
``name`` field, unless overridden by the positional ``NAME`` argument.
|
||||
|
||||
Signature:
|
||||
``agents actor add <NAME> --config <FILE> [--update] [--unsafe]
|
||||
``agents actor add [--config|-c <FILE>] [<NAME>] [--update] [--unsafe]
|
||||
[--set-default] [--option key=value] [--format FORMAT]``
|
||||
|
||||
Examples:
|
||||
agents actor add --config ./actors/my-actor.yaml
|
||||
agents actor add local/my-actor --config ./actors/my-actor.yaml
|
||||
agents actor add local/my-actor --config ./actors/my-actor.yaml --update
|
||||
agents actor add local/my-actor --config actor.yaml --format json
|
||||
agents actor add --config ./actors/my-actor.yaml --update
|
||||
agents actor add --config actor.yaml --format json
|
||||
"""
|
||||
service, registry = _get_services()
|
||||
option_overrides = _parse_option_overrides(option)
|
||||
@@ -597,6 +598,15 @@ def add(
|
||||
assert loaded is not None, "unreachable: config is not None"
|
||||
yaml_text, config_blob = loaded
|
||||
|
||||
# Derive actor name from config when not provided as argument.
|
||||
if name is None:
|
||||
name = config_blob.get("name")
|
||||
if not name:
|
||||
raise typer.BadParameter(
|
||||
"Actor name is required. Provide it as a positional argument "
|
||||
"or as a 'name' field in the config file."
|
||||
)
|
||||
|
||||
# Validate v3 config via ActorConfigSchema if detected.
|
||||
# This ensures v3 actors are fully validated (cycle detection, required
|
||||
# fields, enum values) before the registry stores them.
|
||||
|
||||
Reference in New Issue
Block a user