CHANGELOG.md

@@ -418,7 +418,9 @@ ensuring data is stored with proper parameter values.
 - Fixed stale `AUTO-BUG-POOL` tracking prefix references in automation-tracking.md documentation and agent-system-specification.md spec document, replaced with correct `AUTO-BUG-SUP` prefix used by the bug-hunt-pool-supervisor agent (#7875).
 
 - **`agents session list` now displays full 26-character session ULIDs** (#10970): The Rich table
-  and Summary panel ("Most Recent" / "Oldest") previously showed only the first 8 characters of
+  and Summary panel
+- **`agents actor add` reads name from config file instead of positional argument** (#11047): Removed the mandatory positional ``NAME`` argument from `agents actor add`.  The CLI now reads the actor name from the ``name`` field inside the YAML/JSON config file specified with ``--config``.  A helper function ``_resolve_actor_name()`` validates that the field is present and non-empty, providing a clear error message guiding users to place the name inside the config file. The `agents actor update` command retains its positional ``NAME`` argument since it identifies which registered actor to be modified.
+ ("Most Recent" / "Oldest") previously showed only the first 8 characters of
   each session ULID. This made the output unusable for copy-paste into `session tell`,
   `session show`, `session delete`, and `session export`, all of which require the full
   26-character identifier. The full ULID is now displayed in all output formats (Rich, plain,

CONTRIBUTORS.md

@@ -24,6 +24,7 @@
 # Details
 * HAL 9000 has contributed spec clarifications for layer boundary DI exception, ULID scope, ACMS pipeline contracts, and TUI component interfaces (PR #10451): documented architectural invariants including the DI container exception, clarified ULID identifier scope distinguishing domain entities from internal implementation details, added per-stage protocol contracts for all 10 ACMS pipeline stages with storage tier definitions, budget enforcement protocol, and context assembly output format, and defined public interfaces with verifiable checks for 8 TUI components.
 
+* HAL 9000 has contributed the `agents actor add` configuration file name resolution change (PR #11047): removed positional NAME argument from actor add, implemented `_resolve_actor_name()` helper to read and validate the name from YAML/JSON config files, with clear error messaging guiding users to place name in the config file.
 
 
 Below are some of the specific details of various contributions.

features/actor_add_name_from_config.feature

@@ -0,0 +1,39 @@
+Feature: agents actor add reads actor name from YAML config file
+  As a CleverAgents CLI user
+  I want `agents actor add` to read the actor name from the ``name`` field in the config file
+  So that the name and all other configuration live together in one file, not split between CLI args and config
+
+  Background:
+    Given an actor CLI runner
+
+  @tdd_issue @tdd_issue_11047
+  Scenario: actor add reads name from YAML config successfully
+    Given I have an actor YAML config file with name field "local/my-actor"
+    And a mock registry ready to accept the new actor
+    When I run actor add with only --config flag pointing to the config file
+    Then the actor add should succeed and register "local/my-actor" from config filename
+
+  @tdd_issue @tdd_issue_11047
+  Scenario: actor add reads name from JSON config successfully
+    Given I have an actor JSON config file with name field "local/json-actor"
+    And a mock registry ready to accept the new actor
+    When I run actor add with only --config flag pointing to the config file
+    Then the actor add should succeed and register "local/json-actor" from config filename
+
+  @tdd_issue @tdd_issue_11047
+  Scenario: actor add fails when config has no name field
+    Given I have an actor YAML config file without a name field
+    When I run actor add with only --config flag pointing to the empty config
+    Then the actor command should fail with "Missing required 'name' field"
+
+  @tdd_issue @tdd_issue_11047
+  Scenario: actor add fails when config name is null
+    Given I have an actor YAML config file with null name
+    When I run actor add with only --config flag pointing to the null-name config
+    Then the actor command should fail with "Missing required 'name' field"
+
+  @tdd_issue @tdd_issue_11047
+  Scenario: actor update still accepts positional NAME (names registered actor to modify)
+    Given a registered actor "local/existing-actor" already exists in the registry
+    When I run actor update with "local/existing-actor" as positional argument
+    Then the actor update should succeed for that registered actor

features/actor_add_name_positional.feature

@@ -1,32 +0,0 @@
-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 --config <FILE> [<NAME>]
-
-  @tdd_issue @tdd_issue_4230 @tdd_expected_fail
-  Scenario: actor add accepts NAME as positional argument
-    Given an actor CLI runner
-    And I have an actor JSON config file without a name field
-    When I run actor add with NAME positional argument and config
-    Then the actor add should succeed with the positional name
-
-  @tdd_issue @tdd_issue_4230 @tdd_expected_fail
-  Scenario: actor add NAME positional argument takes precedence over config name
-    Given an actor CLI runner
-    And I have an actor JSON config file with a different name
-    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
-
-  @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 add should fail with a BadParameter error about missing actor name

features/steps/actor_add_name_from_config_steps.py

@@ -0,0 +1,217 @@
+"""Step definitions for actor add reading name from config file."""
+
+from __future__ import annotations
+
+import json
+import tempfile
+from pathlib import Path
+from typing import Any
+from unittest.mock import MagicMock, patch
+
+import yaml
+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
+
+
+def _make_actor(
+    *,
+    name: str = "local/test-actor",
+    provider: str = "openai",
+    model: str = "gpt-4o-mini",
+    config: dict[str, Any] | 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),
+        graph_descriptor=None,
+        unsafe=False,
+        is_built_in=False,
+        is_default=False,
+    )
+
+
+def _register_cleanup(context: Any, path: Path) -> None:
+    if not hasattr(context, "_cleanup_handlers"):
+        context._cleanup_handlers = []
+    context._cleanup_handlers.append(lambda: path.unlink(missing_ok=True))
+
+
+# Given Steps
+
+
+@given('I have an actor YAML config file with name field "{name}"')
+def step_yaml_config_with_name(context, name):
+    yaml_content = (
+        f"name: {name}\n"
+        "type: llm\n"
+        "description: Test actor created by BDD scenario\n"
+        "provider: openai\n"
+        "model: gpt-4o-mini\n"
+    )
+    context.actor_config_data = {"name": name}
+    with tempfile.NamedTemporaryFile(
+        delete=False, suffix=".yaml", mode="w", encoding="utf-8"
+    ) as handle:
+        handle.write(yaml_content)
+        context.actor_config_path = Path(handle.name)
+    _register_cleanup(context, context.actor_config_path)
+
+
+@given('I have an actor JSON config file with name field "{name}"')
+def step_json_config_with_name(context, name):
+    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)
+        context.actor_config_path = Path(handle.name)
+    _register_cleanup(context, context.actor_config_path)
+
+
+@given("a mock registry ready to accept the new actor")
+def step_mock_registry_ready(context):
+    """Marker for scenarios that set up mocking in When steps."""
+    pass
+
+
+@given("I have an actor YAML config file without a name field")
+def step_yaml_no_name_field(context):
+    context.actor_config_data = {
+        "provider": "openai",
+        "model": "gpt-4o-mini",
+    }
+    with tempfile.NamedTemporaryFile(
+        delete=False, suffix=".yaml", mode="w", encoding="utf-8"
+    ) as handle:
+        handle.write(yaml.dump(context.actor_config_data))
+        context.actor_config_path = Path(handle.name)
+    _register_cleanup(context, context.actor_config_path)
+
+
+@given("I have an actor YAML config file with null name")
+def step_yaml_null_name_field(context):
+    context.actor_config_data = {
+        "name": None,
+        "provider": "openai",
+        "model": "gpt-4o-mini",
+    }
+    with tempfile.NamedTemporaryFile(
+        delete=False, suffix=".yaml", mode="w", encoding="utf-8"
+    ) as handle:
+        handle.write(yaml.dump(context.actor_config_data))
+        context.actor_config_path = Path(handle.name)
+    _register_cleanup(context, context.actor_config_path)
+
+
+@given("a pre-existing {name} actor is in the registry")
+def step_existing_actor_registered(context, name):
+    context.existing_actor_name = name
+
+
+# When Steps
+
+
+@when("I run actor add with only --config flag pointing to the config file")
+def step_run_add_with_config_only(context):
+    mock_actor = _make_actor(
+        name=context.actor_config_data.get("name", "local/fallback"),
+        provider=context.actor_config_data.get("provider", "openai"),
+        model=context.actor_config_data.get("model", "gpt-4o-mini"),
+        config=context.actor_config_data,
+    )
+    with (
+        patch("cleveragents.cli.commands.actor._get_services") as mock_get_services,
+    ):
+        registry = MagicMock()
+        registry.upsert_actor.return_value = mock_actor
+        registry.get_actor.side_effect = NotFoundError("not found")
+        mock_get_services.return_value = (MagicMock(), registry)
+
+        context.result = context.runner.invoke(
+            actor_app,
+            ["add", "--config", str(context.actor_config_path)],
+            catch_exceptions=True,
+        )
+        context.mock_registry = registry
+        context.mock_service = MagicMock()
+
+
+@when("I run actor add with only --config flag pointing to the empty config")
+def step_run_add_no_name(context):
+    with patch("cleveragents.cli.commands.actor._get_services") as mock_get_services:
+        mock_get_services.return_value = (MagicMock(), MagicMock())
+
+        context.result = context.runner.invoke(
+            actor_app,
+            ["add", "--config", str(context.actor_config_path)],
+            catch_exceptions=True,
+        )
+
+
+@when("I run actor add with only --config flag pointing to the null-name config")
+def step_run_add_null_name(context):
+    with patch("cleveragents.cli.commands.actor._get_services") as mock_get_services:
+        mock_get_services.return_value = (MagicMock(), MagicMock())
+
+        context.result = context.runner.invoke(
+            actor_app,
+            ["add", "--config", str(context.actor_config_path)],
+            catch_exceptions=True,
+        )
+
+
+@when('I run actor update with "{name}" as positional argument')
+def step_run_update_with_name(context, name):
+    mock_actor = _make_actor(name=name)
+    with patch("cleveragents.cli.commands.actor._get_services") as mock_get_services:
+        registry = MagicMock()
+        registry.get_actor.return_value = mock_actor
+        registry.update_actor.return_value = mock_actor
+        mock_get_services.return_value = (MagicMock(), registry)
+
+        context.result = context.runner.invoke(
+            actor_app,
+            ["update", name],
+            catch_exceptions=True,
+        )
+
+
+# Then Steps
+
+
+@then('the actor add should succeed and register "{name}" from config filename')
+def step_add_succeeds_from_config_name(context, name):
+    assert context.result.exit_code == 0, (
+        f"Expected exit_code=0, got {context.result.exit_code}.\n"
+        f"Output:\n{context.result.output}"
+    )
+    call_kwargs = context.mock_registry.upsert_actor.call_args.kwargs
+    actual_name = call_kwargs.get("name")
+    assert actual_name == name, f"Expected name={name!r}, got {actual_name!r}"
+
+
+@then('the actor command should fail with "{text}"')
+def step_add_fails_with(context, text):
+    assert context.result.exit_code != 0, (
+        f"Expected non-zero exit code, got {context.result.exit_code}.\n"
+        f"Output:\n{context.result.output}"
+    )
+    output = context.result.output.lower()
+    assert text.lower() in output, f"Expected '{text}' in output but got:\n{output}"
+
+
+@then("the actor update should succeed for that registered actor")
+def step_update_succeeds(context):
+    assert context.result.exit_code == 0

features/steps/actor_add_name_positional_steps.py

@@ -1,232 +0,0 @@
-"""Step definitions for actor add NAME positional argument feature."""
-
-from __future__ import annotations
-
-import json
-import tempfile
-from pathlib import Path
-from typing import Any
-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
-
-
-def _make_actor(
-    *,
-    name: str = "local/test-actor",
-    provider: str = "openai",
-    model: str = "gpt-4o-mini",
-    config: dict[str, Any] | None = None,
-    unsafe: bool = False,
-    is_default: bool = False,
-    is_built_in: bool = False,
-) -> Actor:
-    blob = config or {}
-    return Actor(
-        id=1,
-        name=name,
-        provider=provider,
-        model=model,
-        config_blob=blob,
-        config_hash=Actor.compute_hash(blob),
-        graph_descriptor=None,
-        unsafe=unsafe,
-        is_built_in=is_built_in,
-        is_default=is_default,
-    )
-
-
-def _register_cleanup(context: Any, path: Path) -> None:
-    context._cleanup_handlers.append(lambda: path.unlink(missing_ok=True))
-
-
-@given("I have an actor JSON config file without a name field")
-def step_impl(context: Any) -> None:
-    context.actor_config_data = {
-        "provider": "openai",
-        "model": "gpt-4o-mini",
-        "temperature": 0.5,
-    }
-    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)
-
-
-@given("I have an actor JSON config file with a different name")
-def step_impl(context: Any) -> None:
-    context.actor_config_data = {
-        "name": "local/config-name-actor",
-        "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)
-
-
-@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"
-    mock_actor = _make_actor(name=context.positional_name)
-    with patch("cleveragents.cli.commands.actor._get_services") as mock_svc:
-        registry = MagicMock()
-        registry.upsert_actor.return_value = mock_actor
-        mock_svc.return_value = (MagicMock(), registry)
-        context.result = context.runner.invoke(
-            actor_app,
-            [
-                "add",
-                context.positional_name,
-                "--config",
-                str(context.actor_config_path),
-            ],
-        )
-        context.mock_registry = registry
-
-
-@when("I run actor add with NAME positional argument overriding config name")
-def step_impl(context: Any) -> None:
-    context.positional_name = "local/positional-name-actor"
-    mock_actor = _make_actor(name=context.positional_name)
-    with patch("cleveragents.cli.commands.actor._get_services") as mock_svc:
-        registry = MagicMock()
-        registry.upsert_actor.return_value = mock_actor
-        mock_svc.return_value = (MagicMock(), registry)
-        context.result = context.runner.invoke(
-            actor_app,
-            [
-                "add",
-                context.positional_name,
-                "--config",
-                str(context.actor_config_path),
-            ],
-        )
-        context.mock_registry = registry
-
-
-@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,
-            [
-                "add",
-                "--config",
-                str(context.actor_config_path),
-            ],
-        )
-        context.mock_registry = registry
-
-
-@then("the actor add should succeed with the positional 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}"
-    )
-    # Verify the registry was called with the positional name
-    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
-    )
-    assert actual_name == context.positional_name, (
-        f"Expected upsert_actor called with name={context.positional_name!r}, "
-        f"got name={actual_name!r}"
-    )
-
-
-@then("the actor add should use the positional NAME not 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
-    )
-    assert actual_name == context.positional_name, (
-        f"Expected upsert_actor called with positional name={context.positional_name!r}, "
-        f"but got name={actual_name!r} (config name was 'local/config-name-actor')"
-    )
-
-
-@then("the actor command should fail with missing argument error")
-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}"
-    )
-
-
-@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}"
-    )

src/cleveragents/cli/commands/actor.py

@@ -388,6 +388,40 @@ def _parse_option_overrides(option_values: list[str] | None) -> dict[str, Any]:
     return overrides
 
 
+def _resolve_actor_name(
+    config_blob: dict[str, Any],
+    config_path: Path,
+) -> str:
+    """Resolve the actor name from the loaded config blob.
+
+    The ``name`` field is mandatory in v3 YAML schemas and strongly
+    recommended for all actor config formats.  This helper reads it from
+    the parsed config blob and raises a user-friendly error when the field
+    is missing or empty, guiding users to provide the name either positionally
+    or inside the YAML / JSON file.
+
+    Args:
+        config_blob: The already-parsed YAML/JSON configuration dictionary.
+        config_path: Original path to the config file (used in error messages).
+
+    Returns:
+        The actor name string extracted from ``config_blob["name"]``.
+
+    Raises:
+        typer.BadParameter: If ``name`` is absent, null, or empty.
+    """
+    raw_name = config_blob.get("name")
+    if not raw_name or not isinstance(raw_name, str) or not raw_name.strip():
+        raise typer.BadParameter(
+            f"Actor name is required: missing required 'name' field. "
+            f"Provide it either as the positional NAME argument to "
+            f"`agents actor add`, or via a 'name' field inside "
+            f"{config_path} (e.g. ``name: local/my-actor`` at the top of the "
+            "YAML / JSON document)."
+        )
+    return raw_name.strip()
+
+
 def _validate_v3_config(source_path: Path, config_blob: dict[str, Any]) -> None:
     """Validate a v3 config blob using ActorConfigSchema.
 
@@ -578,9 +612,11 @@ def add(
     name: Annotated[
         str | None,
         typer.Argument(
-            help="Namespaced actor name (e.g. local/my-actor). "
-            "If omitted, the name is derived from the config file.",
-            metavar="NAME",
+            help=(
+                "Optional actor name. If omitted, the name is read from the "
+                "'name' field of the --config file. When supplied, it overrides "
+                "the config's name."
+            ),
         ),
     ] = None,
     config: Annotated[
@@ -612,12 +648,13 @@ def add(
 ) -> None:
     """Add a new actor configuration.
 
-    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.
+    The actor name may be specified either positionally (NAME) or via the
+    ``name`` field inside the ``--config`` file. The positional NAME is now
+    optional; when omitted, the name is read from the config file.  When both
+    are present, the positional NAME takes precedence.
 
     Signature:
-    ``agents actor add [--config|-c <FILE>] [<NAME>] [--update] [--unsafe]
+    ``agents actor add [NAME] --config <FILE> [--update] [--unsafe]
     [--set-default] [--option key=value] [--format FORMAT]``
 
     Examples:
@@ -648,14 +685,12 @@ def add(
     if _detect_nested_config_actor(config_blob):
         config_blob = _flatten_config_actor(config_blob)
 
-    # 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."
-            )
+    # Resolve actor name: positional NAME takes precedence over the config's
+    # 'name' field. When both are absent we raise via _resolve_actor_name().
+    if name is not None and name.strip():
+        name = name.strip()
+    else:
+        name = _resolve_actor_name(config_blob, config)
 
     # Validate v3 config via ActorConfigSchema if detected.
     # This ensures v3 actors are fully validated (cycle detection, required
@@ -701,7 +736,7 @@ def add(
             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"
+                " — use --update to replace"
             )
             raise typer.Exit(code=1)
         except typer.Exit: