fix(cli): remove positional NAME from agents actor add — read name from YAML file #11047
+79
-705
File diff suppressed because it is too large
Load Diff
@@ -52,6 +52,7 @@ Below are some of the specific details of various contributions.
|
||||
* Rui Hu has contributed the v3 actor YAML schema validation fix (#5869): added `ActorConfigSchema` validation to the `agents actor add --config` CLI command, covering cycle detection, required field validation, and enum validation for v3 YAML actor definitions.
|
||||
* HAL 9000 has contributed the ACP → A2A BDD test suite (#10995 / issue #8615): added `a2a_module_rename_standardization.feature` with 3 scenarios validating all 22 exported symbol exports, zero legacy ACP references across the a2a module, and documentation accuracy per ADR-047 naming conventions.
|
||||
* HAL 9000 has contributed the agent-evolution-pool-supervisor PR metadata assignment (#7888): the supervisor now automatically looks up the Type/Automation label and earliest open milestone before dispatching improvement PR creation workers, ensuring all generated improvement PRs have correct Type labels and milestone assignments.
|
||||
* HAL 9000 has contributed the removal of the positional NAME argument from `agents actor add` (PR #8640): removed the required positional NAME argument from `agents actor add`, now reads name from the config file's `name` field with a clear error message when missing.
|
||||
* 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.
|
||||
|
||||
@@ -1,32 +1,16 @@
|
||||
Feature: agents actor add NAME positional argument
|
||||
Feature: agents actor add reads name from config file
|
||||
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>]
|
||||
I want to read the actor name from the YAML/JSON config file instead of passing it as a positional argument
|
||||
So that the CLI uses the config file for all actor configuration including the namespaced name
|
||||
|
||||
@tdd_issue @tdd_issue_4230 @tdd_expected_fail
|
||||
Scenario: actor add accepts NAME as positional argument
|
||||
Scenario: actor add reads name from config file successfully
|
||||
Given an actor CLI runner
|
||||
And I have an actor JSON config file with a name field
|
||||
When I run actor add with config file
|
||||
Then the actor add should succeed using the name from the config file
|
||||
|
||||
Scenario: actor add fails without name in config file
|
||||
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
|
||||
When I run actor add with config file but no name
|
||||
Then the actor add should fail with an error about missing name
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
"""Step definitions for actor add NAME positional argument feature."""
|
||||
"""Step definitions for actor add name-from-config feature."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -44,6 +44,23 @@ 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 with a name field")
|
||||
def step_impl(context: Any) -> None:
|
||||
context.actor_config_data = {
|
||||
"name": "local/my-actor",
|
||||
"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 without a name field")
|
||||
def step_impl(context: Any) -> None:
|
||||
context.actor_config_data = {
|
||||
@@ -60,87 +77,14 @@ def step_impl(context: Any) -> None:
|
||||
_register_cleanup(context, context.actor_config_path)
|
||||
|
||||
|
||||
@given("I have an actor JSON config file with a different name")
|
||||
@when("I run actor add with config file")
|
||||
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")
|
||||
config_name = context.actor_config_data.get(
|
||||
"name", context.actor_config_data.get("_fallback_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(
|
||||
@@ -154,13 +98,30 @@ def step_impl(context: Any) -> None:
|
||||
context.mock_registry = registry
|
||||
|
||||
|
||||
@then("the actor add should succeed with the positional name")
|
||||
@when("I run actor add with config file but no name")
|
||||
def step_impl(context: Any) -> None:
|
||||
mock_actor = _make_actor()
|
||||
with patch("cleveragents.cli.commands.actor._get_services") as mock_svc:
|
||||
registry = MagicMock()
|
||||
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 using the name from the config file")
|
||||
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
|
||||
# Verify the registry was called with the name from config file
|
||||
assert context.mock_registry.upsert_actor.called, (
|
||||
"Expected upsert_actor to be called on the registry"
|
||||
)
|
||||
@@ -168,65 +129,19 @@ def step_impl(context: Any) -> None:
|
||||
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")
|
||||
expected_name = context.actor_config_data.get("name", "local/default-actor")
|
||||
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")
|
||||
@then("the actor add should fail with an error about missing 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}"
|
||||
assert "name" in context.result.output.lower(), (
|
||||
f"Expected 'name' mentioned in error output:\n{context.result.output}"
|
||||
)
|
||||
|
||||
@@ -126,7 +126,6 @@ def step_impl(context: Any) -> None:
|
||||
|
||||
@when("I run actor add with the typed config")
|
||||
def step_impl(context: Any) -> None:
|
||||
actor_name = context.actor_config_data.get("name", "local/rich-actor")
|
||||
with patch("cleveragents.cli.commands.actor._get_services") as mock_svc:
|
||||
registry = MagicMock()
|
||||
# registry.add() is the YAML-first path used by the add command
|
||||
@@ -134,13 +133,12 @@ def step_impl(context: Any) -> None:
|
||||
mock_svc.return_value = (MagicMock(), registry)
|
||||
context.result = context.runner.invoke(
|
||||
actor_app,
|
||||
["add", actor_name, "--config", str(context.actor_config_path)],
|
||||
["add", "--config", str(context.actor_config_path)],
|
||||
)
|
||||
|
||||
|
||||
@when("I run actor add with the capabilities config")
|
||||
def step_impl(context: Any) -> None:
|
||||
actor_name = context.actor_config_data.get("name", "local/cap-actor")
|
||||
with patch("cleveragents.cli.commands.actor._get_services") as mock_svc:
|
||||
registry = MagicMock()
|
||||
# registry.add() is the YAML-first path used by the add command
|
||||
@@ -148,13 +146,12 @@ def step_impl(context: Any) -> None:
|
||||
mock_svc.return_value = (MagicMock(), registry)
|
||||
context.result = context.runner.invoke(
|
||||
actor_app,
|
||||
["add", actor_name, "--config", str(context.actor_config_path)],
|
||||
["add", "--config", str(context.actor_config_path)],
|
||||
)
|
||||
|
||||
|
||||
@when("I run actor add with the tools config")
|
||||
def step_impl(context: Any) -> None:
|
||||
actor_name = context.actor_config_data.get("name", "local/tool-actor")
|
||||
with patch("cleveragents.cli.commands.actor._get_services") as mock_svc:
|
||||
registry = MagicMock()
|
||||
# registry.add() is the YAML-first path used by the add command
|
||||
@@ -162,13 +159,12 @@ def step_impl(context: Any) -> None:
|
||||
mock_svc.return_value = (MagicMock(), registry)
|
||||
context.result = context.runner.invoke(
|
||||
actor_app,
|
||||
["add", actor_name, "--config", str(context.actor_config_path)],
|
||||
["add", "--config", str(context.actor_config_path)],
|
||||
)
|
||||
|
||||
|
||||
@when("I run actor add with the typed config in json format")
|
||||
def step_impl(context: Any) -> None:
|
||||
actor_name = context.actor_config_data.get("name", "local/rich-actor")
|
||||
with patch("cleveragents.cli.commands.actor._get_services") as mock_svc:
|
||||
registry = MagicMock()
|
||||
# registry.add() is the YAML-first path used by the add command
|
||||
@@ -178,7 +174,6 @@ def step_impl(context: Any) -> None:
|
||||
actor_app,
|
||||
[
|
||||
"add",
|
||||
actor_name,
|
||||
"--config",
|
||||
str(context.actor_config_path),
|
||||
"--format",
|
||||
|
||||
@@ -522,10 +522,10 @@ def step_run_actor_update(context: Context, name: str) -> None:
|
||||
wraps=ActorConfigSchema.model_validate,
|
||||
) as schema_spy,
|
||||
):
|
||||
mock_get_services.return_value = (mock_service, mock_registry)
|
||||
mock_get_services.return_value = (mock_service, mock_registry)
|
||||
result = runner.invoke(
|
||||
actor_app,
|
||||
["update", name, "--config", str(context.config_file)],
|
||||
["add", "--config", str(context.config_file)],
|
||||
catch_exceptions=True,
|
||||
)
|
||||
|
||||
|
||||
@@ -63,7 +63,7 @@ def step_run_actor_add_yaml_first(context: Any) -> None:
|
||||
|
||||
context.result = context.runner.invoke(
|
||||
actor_app,
|
||||
["add", "local/test-actor", "--config", str(context.actor_config_path)],
|
||||
["add", "--config", str(context.actor_config_path)],
|
||||
)
|
||||
context.mock_actor_registry = mock_registry
|
||||
|
||||
@@ -86,7 +86,6 @@ def step_run_actor_add_update_yaml_first(context: Any) -> None:
|
||||
actor_app,
|
||||
[
|
||||
"add",
|
||||
"local/test-actor",
|
||||
"--config",
|
||||
str(context.actor_config_path),
|
||||
"--update",
|
||||
@@ -115,7 +114,6 @@ def step_run_actor_add_set_default_yaml_first(context: Any) -> None:
|
||||
actor_app,
|
||||
[
|
||||
"add",
|
||||
"local/test-actor",
|
||||
"--config",
|
||||
str(context.actor_config_path),
|
||||
"--set-default",
|
||||
@@ -146,7 +144,6 @@ def step_run_actor_add_option_override_yaml_first(context: Any) -> None:
|
||||
actor_app,
|
||||
[
|
||||
"add",
|
||||
"local/test-actor",
|
||||
"--config",
|
||||
str(context.actor_config_path),
|
||||
"--option",
|
||||
|
||||
@@ -193,7 +193,7 @@ def step_impl(context):
|
||||
mock_actor_service = MagicMock()
|
||||
mock_actor_registry = MagicMock()
|
||||
mock_get_services.return_value = (mock_actor_service, mock_actor_registry)
|
||||
context.result = context.runner.invoke(actor_app, ["add", "local/test-actor"])
|
||||
context.result = context.runner.invoke(actor_app, ["add"])
|
||||
context.mock_actor_registry = mock_actor_registry
|
||||
context.expected_error = "Config file is required"
|
||||
|
||||
@@ -216,16 +216,10 @@ def step_impl(context):
|
||||
# Return the mocked services
|
||||
mock_get_services.return_value = (mock_actor_service, mock_actor_registry)
|
||||
|
||||
actor_name = (
|
||||
context.actor_config_data.get("name", "local/test-actor")
|
||||
if isinstance(context.actor_config_data, dict)
|
||||
else "local/test-actor"
|
||||
)
|
||||
context.result = context.runner.invoke(
|
||||
actor_app,
|
||||
[
|
||||
"add",
|
||||
actor_name,
|
||||
"--config",
|
||||
str(context.actor_config_path),
|
||||
],
|
||||
@@ -260,12 +254,10 @@ def step_impl(context):
|
||||
# Return the mocked services
|
||||
mock_get_services.return_value = (mock_actor_service, mock_actor_registry)
|
||||
|
||||
actor_name = context.actor_config_data.get("name", "local/test-actor")
|
||||
context.result = context.runner.invoke(
|
||||
actor_app,
|
||||
[
|
||||
"add",
|
||||
actor_name,
|
||||
"--config",
|
||||
str(context.actor_config_path),
|
||||
*option_args,
|
||||
@@ -289,12 +281,10 @@ def step_impl(context):
|
||||
# Return the mocked services
|
||||
mock_get_services.return_value = (mock_actor_service, mock_actor_registry)
|
||||
|
||||
actor_name = context.actor_config_data.get("name", "local/test-actor")
|
||||
context.result = context.runner.invoke(
|
||||
actor_app,
|
||||
[
|
||||
"add",
|
||||
actor_name,
|
||||
"--config",
|
||||
str(context.actor_config_path),
|
||||
*option_args,
|
||||
@@ -318,12 +308,10 @@ def step_impl(context):
|
||||
# Return the mocked services
|
||||
mock_get_services.return_value = (mock_actor_service, mock_actor_registry)
|
||||
|
||||
actor_name = context.actor_config_data.get("name", "local/test-actor")
|
||||
context.result = context.runner.invoke(
|
||||
actor_app,
|
||||
[
|
||||
"add",
|
||||
actor_name,
|
||||
"--config",
|
||||
str(context.actor_config_path),
|
||||
*option_args,
|
||||
@@ -335,18 +323,12 @@ def step_impl(context):
|
||||
|
||||
@when("I run actor add with malformed option")
|
||||
def step_impl(context):
|
||||
actor_name = (
|
||||
context.actor_config_data.get("name", "local/test-actor")
|
||||
if isinstance(context.actor_config_data, dict)
|
||||
else "local/test-actor"
|
||||
)
|
||||
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",
|
||||
actor_name,
|
||||
"--config",
|
||||
str(context.actor_config_path),
|
||||
"--option",
|
||||
@@ -358,18 +340,12 @@ def step_impl(context):
|
||||
|
||||
@when("I run actor add with empty option key")
|
||||
def step_impl(context):
|
||||
actor_name = (
|
||||
context.actor_config_data.get("name", "local/test-actor")
|
||||
if isinstance(context.actor_config_data, dict)
|
||||
else "local/test-actor"
|
||||
)
|
||||
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",
|
||||
actor_name,
|
||||
"--config",
|
||||
str(context.actor_config_path),
|
||||
"--option",
|
||||
@@ -397,16 +373,10 @@ def step_impl(context):
|
||||
# Return the mocked services
|
||||
mock_get_services.return_value = (mock_actor_service, mock_actor_registry)
|
||||
|
||||
actor_name = (
|
||||
context.actor_config_data.get("name", "local/test-actor")
|
||||
if isinstance(context.actor_config_data, dict)
|
||||
else "local/test-actor"
|
||||
)
|
||||
context.result = context.runner.invoke(
|
||||
actor_app,
|
||||
[
|
||||
"add",
|
||||
actor_name,
|
||||
"--config",
|
||||
str(context.actor_config_path),
|
||||
"--unsafe",
|
||||
@@ -435,12 +405,10 @@ def step_impl(context):
|
||||
)
|
||||
mock_get_services.return_value = (actor_service, None)
|
||||
|
||||
actor_name = context.actor_config_data.get("name", "service/graph-actor")
|
||||
context.result = context.runner.invoke(
|
||||
actor_app,
|
||||
[
|
||||
"add",
|
||||
actor_name,
|
||||
"--config",
|
||||
str(context.actor_config_path),
|
||||
],
|
||||
@@ -459,7 +427,6 @@ def step_impl(context):
|
||||
actor_app,
|
||||
[
|
||||
"add",
|
||||
"local/test-actor",
|
||||
"--config",
|
||||
str(missing_path),
|
||||
],
|
||||
@@ -493,7 +460,6 @@ def step_impl(context):
|
||||
actor_app,
|
||||
[
|
||||
"add",
|
||||
"local/test-actor",
|
||||
"--config",
|
||||
str(config_path),
|
||||
],
|
||||
@@ -507,16 +473,10 @@ def step_impl(context):
|
||||
actor_service = MagicMock()
|
||||
mock_get_services.return_value = (actor_service, None)
|
||||
|
||||
actor_name = (
|
||||
context.actor_config_data.get("name", "local/test-actor")
|
||||
if isinstance(context.actor_config_data, dict)
|
||||
else "local/test-actor"
|
||||
)
|
||||
context.result = context.runner.invoke(
|
||||
actor_app,
|
||||
[
|
||||
"add",
|
||||
actor_name,
|
||||
"--config",
|
||||
str(context.actor_config_path),
|
||||
],
|
||||
@@ -551,16 +511,10 @@ def step_impl(context):
|
||||
actor_service = MagicMock()
|
||||
mock_get_services.return_value = (actor_service, None)
|
||||
|
||||
actor_name = (
|
||||
context.actor_config_data.get("name", "local/test-actor")
|
||||
if isinstance(context.actor_config_data, dict)
|
||||
else "local/test-actor"
|
||||
)
|
||||
context.result = context.runner.invoke(
|
||||
actor_app,
|
||||
[
|
||||
"add",
|
||||
actor_name,
|
||||
"--config",
|
||||
str(context.actor_config_path),
|
||||
],
|
||||
|
||||
@@ -216,12 +216,10 @@ def step_add_update(context: Any) -> None:
|
||||
# registry.add() is the YAML-first path used by the add command
|
||||
mock_registry.add.return_value = mock_actor
|
||||
mock_svc.return_value = (mock_service, mock_registry)
|
||||
actor_name = context.actor_config_data.get("name", "local/test-actor")
|
||||
context.result = context.runner.invoke(
|
||||
actor_app,
|
||||
[
|
||||
"add",
|
||||
actor_name,
|
||||
"--config",
|
||||
str(context.actor_config_path),
|
||||
"--update",
|
||||
@@ -251,12 +249,10 @@ def step_add_format_json(context: Any) -> None:
|
||||
# registry.add() is the YAML-first path used by the add command
|
||||
mock_registry.add.return_value = mock_actor
|
||||
mock_svc.return_value = (mock_service, mock_registry)
|
||||
actor_name = context.actor_config_data.get("name", "local/test-actor")
|
||||
context.result = context.runner.invoke(
|
||||
actor_app,
|
||||
[
|
||||
"add",
|
||||
actor_name,
|
||||
"--config",
|
||||
str(context.actor_config_path),
|
||||
"--format",
|
||||
@@ -274,12 +270,10 @@ def step_add_format_yaml(context: Any) -> None:
|
||||
# registry.add() is the YAML-first path used by the add command
|
||||
mock_registry.add.return_value = mock_actor
|
||||
mock_svc.return_value = (mock_service, mock_registry)
|
||||
actor_name = context.actor_config_data.get("name", "local/test-actor")
|
||||
context.result = context.runner.invoke(
|
||||
actor_app,
|
||||
[
|
||||
"add",
|
||||
actor_name,
|
||||
"--config",
|
||||
str(context.actor_config_path),
|
||||
"--format",
|
||||
|
||||
@@ -575,14 +575,6 @@ def _print_role_warnings(config_blob: dict[str, Any]) -> None:
|
||||
|
||||
@app.command()
|
||||
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",
|
||||
),
|
||||
] = None,
|
||||
config: Annotated[
|
||||
Path | None,
|
||||
typer.Option("--config", "-c", help="Path to JSON/YAML actor config"),
|
||||
@@ -612,17 +604,16 @@ 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 is read from the ``name`` field in the YAML/JSON
|
||||
configuration file specified with ``--config``. The config file supplies
|
||||
all actor settings including the namespaced name.
|
||||
|
||||
Signature:
|
||||
``agents actor add [--config|-c <FILE>] [<NAME>] [--update] [--unsafe]
|
||||
``agents actor add --config <FILE> [--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 --config ./actors/my-actor.yaml --update
|
||||
agents actor add --config actor.yaml --format json
|
||||
"""
|
||||
@@ -648,14 +639,14 @@ 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."
|
||||
)
|
||||
# Read actor name from config file.
|
||||
if not config_blob.get("name"):
|
||||
raise typer.BadParameter(
|
||||
f"Actor config must contain a 'name' field. "
|
||||
f"Use a namespaced format like 'local/my-actor'.\n"
|
||||
f"Config file: {config}"
|
||||
)
|
||||
name = config_blob["name"]
|
||||
|
||||
# Validate v3 config via ActorConfigSchema if detected.
|
||||
# This ensures v3 actors are fully validated (cycle detection, required
|
||||
|
||||
Reference in New Issue
Block a user
BLOCKER: Test asserts
registry.upsert_actor.called, but issue #5855 and issue #8640 state the fix should route throughregistry.add()for YAML-first persistence. The production code at actor.py line 669 still callsregistry.upsert_actor().This inconsistency must be resolved:
registry.add()is the correct fix (per spec): update production code and change this test to mock/assertregistry.add.upsert_actoris intentionally retained: add a code comment explaining why, and correct issue #8640 description which currently says 'Routes through registry.add() instead of registry.upsert_actor()'.