fix(actor): handle nested actor type and combined config.actor field in v3 YAML
CI / push-validation (push) Successful in 43s
CI / helm (push) Successful in 50s
CI / benchmark-regression (push) Failing after 1m28s
CI / build (push) Successful in 1m39s
CI / quality (push) Successful in 2m32s
CI / lint (push) Successful in 2m37s
CI / security (push) Successful in 2m40s
CI / typecheck (push) Successful in 2m47s
CI / e2e_tests (push) Successful in 2m43s
CI / integration_tests (push) Successful in 5m0s
CI / unit_tests (push) Successful in 8m3s
CI / docker (push) Successful in 1m52s
CI / coverage (push) Successful in 14m30s
CI / status-check (push) Successful in 6s
CI / helm (pull_request) Successful in 46s
CI / build (pull_request) Successful in 1m22s
CI / quality (pull_request) Successful in 1m39s
CI / lint (pull_request) Successful in 1m55s
CI / security (pull_request) Successful in 1m55s
CI / push-validation (pull_request) Successful in 1m40s
CI / typecheck (pull_request) Successful in 1m42s
CI / integration_tests (pull_request) Successful in 4m29s
CI / unit_tests (pull_request) Successful in 5m42s
CI / docker (pull_request) Successful in 1m38s
CI / coverage (pull_request) Successful in 10m50s
CI / status-check (pull_request) Successful in 4s
CI / benchmark-publish (push) Successful in 1h24m10s

Two defects in ActorConfiguration._extract_v3_actor() prevented YAML files
using the spec-canonical nested actors map format from being registered:

Defect A: The method looked for the 'type' field inside the 'config:' block
(config_block.get('type')) instead of at the actor-entry level
(first_entry.get('type')). Because the spec places 'type' at
actors.<name>.type — a sibling of 'config:', not a child of it — the v3
detection branch never fired for nested-actors-map YAMLs, causing
_extract_v3_actor() to return (None, None, None, False) immediately.

Defect B: Even if the type was found, the method only consulted separate
config.provider and config.model keys; it never parsed the combined
config.actor: 'provider/model' shorthand. Without the combined-field
fallback, provider and model remained None for all spec-canonical YAMLs
that use the shorthand, causing from_blob() to raise
'BadParameter: provider is required'.

Both defects had to be fixed together because Defect A blocked the code
from ever reaching the path where Defect B would otherwise have been
encountered.

The combined-format parser (config.actor split on first '/') is inserted
after the explicit config.provider / config.model lookups so that explicit
separate fields always take precedence over the shorthand when both are
present.

Parsing logic extracted into _parse_combined_actor_field() helper to
eliminate DRY violation. Both provider and model halves validated
consistently (empty provider half was previously silently inferred).

Adds Behave scenarios covering: nested actors map + combined config.actor
(success), explicit config.provider precedence over config.actor,
explicit config.model precedence over config.actor, genuinely missing
provider/model raising validation error, and malformed combined values
(empty model half, empty provider half, missing delimiter). Adds a Robot
integration test for the combined-actor YAML path with provider/model
assertions via show. Restores accidentally deleted TOOL and GRAPH Robot
integration tests. Mirrors provider/model extraction in
step_run_actor_update for assertion step compatibility.

ISSUES CLOSED: #11189
This commit was merged in pull request #11193.
This commit is contained in:
2026-05-13 10:40:53 +00:00
committed by Forgejo
parent 8c5dbf3f33
commit cc8e013f9b
5 changed files with 412 additions and 39 deletions
+8
View File
@@ -54,6 +54,14 @@ Changed `wf10_batch.robot` to be less likely to create files, and
## [Unreleased]
- **Fixed `agents actor add --config` crash with nested `actors:` map and `config.actor` combined shorthand** (#11189): The
CLI command `agents actor add --config` now correctly parses spec-canonical YAML using the
nested `actors:` map format with `config.actor: "provider/model"` combined shorthand.
Fixed two defects in `ActorConfiguration._extract_v3_actor()`: `type` is now detected at
the actor-entry level (sibling of `config`), and `config.actor` is parsed as fallback
when separate `provider`/`model` keys are absent. Added validation to reject malformed
combined values with empty provider or model halves.
- **`task-implementor` posts work-started notification comments** (#11031): Both
the `issue_impl` and `pr_fix` procedures now post an informational "work
started" comment to the Forgejo issue/PR before beginning implementation.
@@ -271,3 +271,62 @@ Feature: Actor add CLI validates v3 YAML via ActorConfigSchema
When I run the actor add command for "local/bad-edge-ref" with the prepared config file
Then v3actor the command should fail
And v3actor the error message should contain "not found"
# ────────────────────────────────────────────────────────────
# Nested actors map: combined config.actor field (bug #11189)
# ────────────────────────────────────────────────────────────
@tdd_issue @tdd_issue_11189
Scenario: Register LLM actor using nested actors map with combined config.actor field
Given a nested LLM actor YAML using combined config.actor field with name "local/nested-combined-llm"
When I run the actor add command for "local/nested-combined-llm" with the prepared config file
Then v3actor the command should succeed
And v3actor the actor should be registered with provider "anthropic" and model "claude-sonnet-4-5"
Scenario: Explicit config.provider takes precedence over provider derived from config.actor
Given a nested LLM actor YAML with explicit config.provider overriding config.actor with name "local/explicit-provider-llm"
When I run the actor add command for "local/explicit-provider-llm" with the prepared config file
Then v3actor the command should succeed
And v3actor the actor should be registered with provider "explicit-provider"
And v3actor the actor should be registered with model "gpt-4"
Scenario: Nested actors map with no provider or model still raises validation error
Given a nested LLM actor YAML with no provider or model fields with name "local/no-provider-nested-llm"
When I run the actor add command for "local/no-provider-nested-llm" with the prepared config file
Then v3actor the command should fail
And v3actor the error message should contain "provider"
Scenario: Explicit config.model takes precedence over model derived from config.actor
Given a nested LLM actor YAML with explicit config.model overriding config.actor with name "local/explicit-model-llm"
When I run the actor add command for "local/explicit-model-llm" with the prepared config file
Then v3actor the command should succeed
And v3actor the actor should be registered with provider "anthropic"
And v3actor the actor should be registered with model "explicit-model"
# ────────────────────────────────────────────────────────────
# Malformed config.actor edge cases
# ────────────────────────────────────────────────────────────
Scenario: Combined config.actor with empty model half is treated as absent
Given a nested LLM actor YAML with combined config.actor "provider/" and name "local/empty-model-actor"
When I run the actor add command for "local/empty-model-actor" with the prepared config file
Then v3actor the command should fail
And v3actor the error message should contain "provider"
Scenario: Combined config.actor with empty provider half is treated as missing provider
Given a nested LLM actor YAML with combined config.actor "/model" and name "local/empty-provider-actor"
When I run the actor add command for "local/empty-provider-actor" with the prepared config file
Then v3actor the command should fail
And v3actor the error message should contain "provider"
Scenario: Combined config.actor without slash delimiter is treated as absent
Given a nested LLM actor YAML with combined config.actor "noslash" and name "local/noslash-actor"
When I run the actor add command for "local/noslash-actor" with the prepared config file
Then v3actor the command should fail
And v3actor the error message should contain "provider"
Scenario: Combined config.actor with both halves empty is treated as absent
Given a nested LLM actor YAML with combined config.actor "/" and name "local/slash-only-actor"
When I run the actor add command for "local/slash-only-actor" with the prepared config file
Then v3actor the command should fail
And v3actor the error message should contain "provider"
@@ -440,15 +440,22 @@ def step_run_actor_add(context: Context, name: str) -> None:
assert result.exception is None, f"Unexpected exception: {result.exception}"
context.command_error = None
context.actor_registered = True
# Verify registry was called and extract the actor_type from the config
# Verify registry was called and extract actor metadata from the config blob
if mock_registry.upsert_actor.called:
call_kwargs = mock_registry.upsert_actor.call_args
blob = call_kwargs.kwargs.get("config_blob") if call_kwargs.kwargs else None
context.registered_actor_type = (
blob.get("type") if isinstance(blob, dict) else None
)
if isinstance(blob, dict):
context.registered_actor_type = blob.get("type")
context.registered_actor_provider = blob.get("provider")
context.registered_actor_model = blob.get("model")
else:
context.registered_actor_type = None
context.registered_actor_provider = None
context.registered_actor_model = None
else:
context.registered_actor_type = None
context.registered_actor_provider = None
context.registered_actor_model = None
else:
assert isinstance(result.exception, SystemExit), (
f"Expected SystemExit, got: {result.exception}"
@@ -456,6 +463,8 @@ def step_run_actor_add(context: Context, name: str) -> None:
context.command_error = result.output
context.actor_registered = False
context.registered_actor_type = None
context.registered_actor_provider = None
context.registered_actor_model = None
@when('I run the actor update command for "{name}" with the prepared config file')
@@ -532,15 +541,22 @@ def step_run_actor_update(context: Context, name: str) -> None:
assert result.exception is None, f"Unexpected exception: {result.exception}"
context.command_error = None
context.actor_registered = True
# Verify registry was called and extract the actor_type from the config
# Verify registry was called and extract actor metadata from the config blob
if mock_registry.upsert_actor.called:
call_kwargs = mock_registry.upsert_actor.call_args
blob = call_kwargs.kwargs.get("config_blob") if call_kwargs.kwargs else None
context.registered_actor_type = (
blob.get("type") if isinstance(blob, dict) else None
)
if isinstance(blob, dict):
context.registered_actor_type = blob.get("type")
context.registered_actor_provider = blob.get("provider")
context.registered_actor_model = blob.get("model")
else:
context.registered_actor_type = None
context.registered_actor_provider = None
context.registered_actor_model = None
else:
context.registered_actor_type = None
context.registered_actor_provider = None
context.registered_actor_model = None
else:
assert isinstance(result.exception, SystemExit), (
f"Expected SystemExit, got: {result.exception}"
@@ -548,6 +564,8 @@ def step_run_actor_update(context: Context, name: str) -> None:
context.command_error = result.output
context.actor_registered = False
context.registered_actor_type = None
context.registered_actor_provider = None
context.registered_actor_model = None
# ── Then Steps (Assertions) — prefixed with "v3actor" to avoid conflicts ─────
@@ -1127,6 +1145,206 @@ route:
context.config_file = _write_yaml_file(context, "bad-edge-ref", yaml_content)
# ── Nested actors map: combined config.actor field (bug #11189) ───────────────
@given('a nested LLM actor YAML using combined config.actor field with name "{name}"')
def step_nested_llm_combined_actor(context: Context, name: str) -> None:
"""Create a spec-compliant nested-actors-map YAML using the combined config.actor field.
The ``actors:`` map format places ``type: llm`` at the actor-entry level
(``actors.<name>.type``), not inside the ``config:`` block. The provider
and model are supplied via the combined ``config.actor: provider/model``
shorthand rather than separate ``config.provider``/``config.model`` keys.
This is the exact format described in the spec and reproduced in bug #11189.
"""
yaml_content = f"""\
name: {name}
cleveragents:
version: "3.0"
default_actor: main
actors:
main:
type: llm
config:
actor: anthropic/claude-sonnet-4-5
system_prompt: "You are a test actor."
temperature: 0.3
"""
context.config_file = _write_yaml_file(context, name, yaml_content)
@given(
"a nested LLM actor YAML with explicit config.provider overriding config.actor"
' with name "{name}"'
)
def step_nested_llm_explicit_provider_overrides_combined(
context: Context, name: str
) -> None:
"""Create a nested-actors-map YAML with both config.provider and config.actor set.
When ``config.provider`` is explicitly present it must take precedence over
the provider parsed from the combined ``config.actor: provider/model``
shorthand. The model half of the combined field is still used as the
model fallback (since no separate ``config.model`` is provided here).
"""
yaml_content = f"""\
name: {name}
cleveragents:
version: "3.0"
default_actor: main
actors:
main:
type: llm
config:
provider: explicit-provider
actor: override/gpt-4
system_prompt: "Precedence test actor."
"""
context.config_file = _write_yaml_file(context, name, yaml_content)
@given('a nested LLM actor YAML with no provider or model fields with name "{name}"')
def step_nested_llm_no_provider_or_model(context: Context, name: str) -> None:
"""Create a nested-actors-map YAML with type: llm but no provider or model.
When neither ``config.provider``, ``config.model``, nor ``config.actor``
are present the extraction must fail gracefully and propagate a validation
error message so that users receive a clear diagnostic.
"""
yaml_content = f"""\
name: {name}
cleveragents:
version: "3.0"
default_actor: main
actors:
main:
type: llm
config:
system_prompt: "Actor without provider or model."
"""
context.config_file = _write_yaml_file(context, name, yaml_content)
@given(
"a nested LLM actor YAML with explicit config.model overriding config.actor"
' with name "{name}"'
)
def step_nested_llm_explicit_model_overrides_combined(
context: Context, name: str
) -> None:
"""Create a nested-actors-map YAML with both config.model and config.actor set.
When ``config.model`` is explicitly present it must take precedence over
the model parsed from the combined ``config.actor: provider/model``
shorthand. The provider half of the combined field is still used as the
provider fallback (since no separate ``config.provider`` is provided here).
"""
yaml_content = f"""\
name: {name}
cleveragents:
version: "3.0"
default_actor: main
actors:
main:
type: llm
config:
model: explicit-model
actor: anthropic/claude-sonnet-4-5
system_prompt: "Model precedence test actor."
"""
context.config_file = _write_yaml_file(context, name, yaml_content)
@given(
'a nested LLM actor YAML with combined config.actor "{combined_value}"'
' and name "{name}"'
)
def step_nested_llm_malformed_combined_actor(
context: Context, combined_value: str, name: str
) -> None:
"""Create a nested-actors-map YAML with a specific config.actor value.
Used to test edge cases of the combined ``config.actor: provider/model``
shorthand: empty model half (``provider/``), empty provider half
(``/model``), no delimiter (``noslash``), empty string, etc.
"""
yaml_content = f"""\
name: {name}
cleveragents:
version: "3.0"
default_actor: main
actors:
main:
type: llm
config:
actor: {combined_value}
system_prompt: "Malformed actor test."
"""
context.config_file = _write_yaml_file(context, name, yaml_content)
@then(
'v3actor the actor should be registered with provider "{provider}" and model "{model}"'
)
def step_actor_registered_with_provider_and_model(
context: Context, provider: str, model: str
) -> None:
"""Assert the actor was registered with the expected provider and model.
Inspects ``registered_actor_provider`` and ``registered_actor_model``
captured by the ``@when`` step from the ``config_blob`` passed to
``registry.upsert_actor()`` after CLI canonicalization. Canonicalization
calls ``setdefault`` so the canonical blob always carries the resolved
provider and model even when they were not present in the original YAML.
"""
assert context.actor_registered, (
"Actor was not registered. Output: "
f"{context.command_result.get('output', '') if context.command_result else ''}"
)
reg_provider = getattr(context, "registered_actor_provider", None)
reg_model = getattr(context, "registered_actor_model", None)
assert reg_provider == provider, (
f"Expected provider '{provider}' but got '{reg_provider}'"
)
assert reg_model == model, f"Expected model '{model}' but got '{reg_model}'"
@then('v3actor the actor should be registered with provider "{provider}"')
def step_actor_registered_with_provider(context: Context, provider: str) -> None:
"""Assert the actor was registered with the expected provider.
Checks ``registered_actor_provider`` captured by the ``@when`` step from
the ``config_blob`` passed to ``registry.upsert_actor()`` after
CLI canonicalization.
"""
assert context.actor_registered, (
"Actor was not registered. Output: "
f"{context.command_result.get('output', '') if context.command_result else ''}"
)
reg_provider = getattr(context, "registered_actor_provider", None)
assert reg_provider == provider, (
f"Expected provider '{provider}' but got '{reg_provider}'"
)
@then('v3actor the actor should be registered with model "{model}"')
def step_actor_registered_with_model(context: Context, model: str) -> None:
"""Assert the actor was registered with the expected model.
Checks ``registered_actor_model`` captured by the ``@when`` step from
the ``config_blob`` passed to ``registry.upsert_actor()`` after
CLI canonicalization.
"""
assert context.actor_registered, (
"Actor was not registered. Output: "
f"{context.command_result.get('output', '') if context.command_result else ''}"
)
reg_model = getattr(context, "registered_actor_model", None)
assert reg_model == model, f"Expected model '{model}' but got '{reg_model}'"
# ── Helper Functions ──────────────────────────────────────────────────────────
@@ -404,3 +404,37 @@ Reject v3 GRAPH Actor With Unreachable Node
Log ${result.stderr}
Should Be Equal As Integers ${result.rc} 1
Should Contain ${result.stderr} unreachable
Register LLM Actor With Nested Actors Map And Combined config.actor Field
[Documentation]
... Verify that agents actor add accepts spec-compliant YAML using the
... nested actors map format with type at actor-entry level and provider/model
... supplied via the combined config.actor: "provider/model" shorthand.
... This is the regression test for bug #11189.
[Tags] slow
${yaml_file}= Set Variable ${TEMPDIR}${/}nested_combined_actor.yaml
${yaml_content}= Catenate SEPARATOR=\n
... name: local/nested-combined-llm
... cleveragents:
... ${SPACE}${SPACE}version: "3.0"
... ${SPACE}${SPACE}default_actor: main
... actors:
... ${SPACE}${SPACE}main:
... ${SPACE}${SPACE}${SPACE}${SPACE}type: llm
... ${SPACE}${SPACE}${SPACE}${SPACE}config:
... ${SPACE}${SPACE}${SPACE}${SPACE}${SPACE}${SPACE}actor: anthropic/claude-sonnet-4-5
... ${SPACE}${SPACE}${SPACE}${SPACE}${SPACE}${SPACE}system_prompt: "You are a test actor."
... ${SPACE}${SPACE}${SPACE}${SPACE}${SPACE}${SPACE}temperature: 0.3
Create File ${yaml_file} ${yaml_content}
${result}= Run Process ${PYTHON} ${HELPER} add local/nested-combined-llm ${yaml_file} cwd=${WORKSPACE}
Log ${result.stdout}
Log ${result.stderr}
Should Be Equal As Integers ${result.rc} 0
Should Contain ${result.stdout} actor-add-success
${show_result}= Run Process ${PYTHON} ${HELPER} show local/nested-combined-llm cwd=${WORKSPACE}
Log ${show_result.stdout}
Log ${show_result.stderr}
Should Be Equal As Integers ${show_result.rc} 0
Should Contain ${show_result.stdout} actor-show-success
Should Contain ${show_result.stdout} anthropic
Should Contain ${show_result.stdout} claude-sonnet-4-5
+85 -31
View File
@@ -12,6 +12,41 @@ from cleveragents.actor.yaml_loader import load_yaml_text
_V3_ACTOR_TYPES: frozenset[str] = frozenset({"llm", "graph", "tool"})
def _parse_combined_actor_field(data: dict[str, Any]) -> tuple[str | None, str | None]:
"""Extract (provider, model) from the combined ``config.actor`` shorthand.
Looks for ``config.actor: "provider/model"`` inside the first entry of
the nested ``actors:`` map. Returns ``(None, None)`` when the shorthand
is absent or malformed (empty provider or model half, missing delimiter).
Args:
data: Top-level actor YAML blob (may contain an ``actors:`` map).
Returns:
``(provider_value, model_value)`` both ``None`` when the combined
field is not present or cannot be parsed.
"""
actors_val = data.get("actors")
if not isinstance(actors_val, dict) or not actors_val:
return None, None
for _, first_entry in actors_val.items():
if not isinstance(first_entry, dict):
continue
config_block = first_entry.get("config")
if isinstance(config_block, dict):
combined = config_block.get("actor")
if isinstance(combined, str) and "/" in combined and combined.strip():
parts = combined.strip().split("/", 1)
provider = parts[0].strip()
model = parts[1].strip()
if provider and model:
return provider, model
break # Only examine the first dict entry in the actors map.
return None, None
class ActorConfiguration(BaseModel):
"""Canonical actor configuration parsed from user-provided blobs."""
@@ -159,12 +194,14 @@ class ActorConfiguration(BaseModel):
The v3 schema is identified by a top-level ``type`` key whose value
is one of ``llm``, ``graph``, or ``tool`` OR by a top-level
``actors:`` map containing at least one entry whose ``config:`` block
``actors:`` map containing at least one entry whose actor-entry level
carries a ``type`` field with one of those values.
Provider and model may appear at the top level OR nested inside
``actors.<actor-name>.config``. When found in the nested config,
they take precedence over top-level values.
``actors.<actor-name>.config`` (either as separate ``provider``/``model``
keys, or as a combined ``actor: "provider/model"`` shorthand). Explicit
``config.provider`` and ``config.model`` take precedence over the combined
shorthand when both are present.
For ``type: graph`` actors the ``route`` block is wrapped into a
graph descriptor.
@@ -179,17 +216,16 @@ class ActorConfiguration(BaseModel):
if isinstance(actors_val, dict) and actors_val:
for _, first_entry in actors_val.items():
if isinstance(first_entry, dict):
config_block = first_entry.get("config")
if isinstance(config_block, dict):
nested_type = config_block.get("type")
is_v3_type = (
isinstance(nested_type, str)
and nested_type.lower() in _V3_ACTOR_TYPES
)
if is_v3_type:
actor_type = nested_type
break
break
# The v3 spec places 'type' at the actor-entry level
# (sibling of 'config'), not inside the config block.
nested_type = first_entry.get("type")
is_v3_type = (
isinstance(nested_type, str)
and nested_type.lower() in _V3_ACTOR_TYPES
)
if is_v3_type:
actor_type = nested_type
break
if not isinstance(actor_type, str):
return None, None, None, False
@@ -197,8 +233,16 @@ class ActorConfiguration(BaseModel):
provider_value = data.get("provider")
unsafe_flag = bool(data.get("unsafe", False))
if not model_value or not isinstance(model_value, str):
if actor_type.lower() != "tool":
# Derive provider/model from nested actors map when top-level fields
# are absent. Separate config.provider / config.model keys take
# precedence over the combined config.actor shorthand.
if (not model_value or not isinstance(model_value, str)) or (
not provider_value or not isinstance(provider_value, str)
):
combined_provider, combined_model = _parse_combined_actor_field(data)
if not isinstance(model_value, str) or not model_value:
# First, check for a separate config.model key in the nested map.
model_from_separate: str | None = None
actors_val = data.get("actors")
if isinstance(actors_val, dict) and actors_val:
for _, first_entry in actors_val.items():
@@ -207,27 +251,37 @@ class ActorConfiguration(BaseModel):
if isinstance(config_block, dict):
mv = config_block.get("model")
if isinstance(mv, str) and mv:
model_value = mv
model_from_separate = mv
break
break
break
if model_from_separate:
model_value = model_from_separate
elif combined_model:
model_value = combined_model
if not isinstance(provider_value, str) or not provider_value:
# First, check for a separate config.provider key in the nested map.
provider_from_separate: str | None = None
actors_val = data.get("actors")
if isinstance(actors_val, dict) and actors_val:
for _, first_entry in actors_val.items():
if isinstance(first_entry, dict):
config_block = first_entry.get("config")
if isinstance(config_block, dict):
pv = config_block.get("provider")
if isinstance(pv, str) and pv:
provider_from_separate = pv
break
break
if provider_from_separate:
provider_value = provider_from_separate
elif combined_provider:
provider_value = combined_provider
if not model_value:
if actor_type.lower() != "tool":
return None, None, None, False
model_value = ""
if not provider_value or not isinstance(provider_value, str):
actors_val = data.get("actors")
if isinstance(actors_val, dict) and actors_val:
for _, first_entry in actors_val.items():
if isinstance(first_entry, dict):
config_block = first_entry.get("config")
if isinstance(config_block, dict):
pv = config_block.get("provider")
if isinstance(pv, str) and pv:
provider_value = pv
break
break
if not provider_value:
provider_value = (
infer_provider_from_model(model_value) if model_value else "custom"