forked from cleveragents/cleveragents-core
f829e68911
The ActorRegistry.add() method rejected spec-compliant YAML that uses the
actors: map format with nested config: blocks because it only looked for
provider/model at the top level of the blob. Four changes fix this:
1. _extract_v2_actor() now handles both the spec-canonical actors: key and
the legacy agents: key, with actors: taking precedence. It also supports
the combined actor field format (e.g. "openai/gpt-4") from the spec.
2. _extract_v2_options() mirrors the same actors:/agents: support.
3. registry.add() now unconditionally calls _extract_v2_actor() so that
nested unsafe flags and graph descriptors are always captured — even
when top-level provider/model are present. This eliminates the
behavioural asymmetry with from_blob().
4. The unsafe confirmation gate now runs before the duplicate-actor check,
and the graph_descriptor resolution uses explicit is-not-None checks
to distinguish "not set" from "set to empty dict".
Review fixes (cycle 4):
- Added 9 new Behave scenarios: _extract_v2_options edge cases (empty map,
None, list, missing options key), _extract_v2_actor with unsafe=True,
add() with missing name field, top-level unsafe: true (rejection +
acceptance), and multi-actor unsafe limitation documentation.
- Added graph descriptor assertions to all _extract_v2_actor direct
scenarios that were missing them.
- Fixed unsafe field coercion to use explicit boolean check (is True or
== 1) instead of bool() to prevent truthy non-boolean values like
"no" from being treated as unsafe.
- Added legacy graph key fallback (blob.get("graph")) in add() for
consistency with from_blob().
- Fixed _StubActorService.upsert_actor to handle set_default parameter
and pass non-None config_blob to Actor.compute_hash().
- Updated stale CLI comment about registry.add() capabilities.
- Applied ruff format to step definitions.
Review fixes (cycle 5 — external review):
- T1/T2/T3: Added unsafe coercion edge-case tests for 1.0 (float→True),
2 (int>1→False), and 0 (int zero→False).
- T4/T5/T6: Added _extract_v2_options tests for non-dict first entry,
missing config block, and actors: preference over agents:.
- T7: Added integration test for top-level graph_descriptor key.
- T8: Added integration tests for top-level provider_type/model_id aliases.
- T9: Added integration tests for top-level unsafe string coercion ("yes"/"no").
- S1: effective_unsafe now includes unsafe/allow_unsafe params to match
from_blob() and spec rule.
- S2: CHANGELOG entry expanded with multi-actor rejection, options
preservation, and coercion fix.
- B1: Changed v2_options guard from truthiness to is-not-None check.
- P1: _extract_v2_options() now returns a shallow copy.
- P2: Nested options overwrite non-dict top-level options values.
- Q3: Fixed comment variable name (unsafe_raw → top_unsafe_raw).
- S3: Added spec-extension comment for provider_type/model_id aliases.
- S4/B3: Cached actors_raw in multi-actor guard; added cross-ref comment.
- Q5: Added TODO(#10832) to _StubActorService duplicate.
- Q6: Replaced misleading (M2) label with (unsafe coercion).
Review fixes (cycle 6 — agent review):
- MAJ-1: Removed allow_unsafe from effective_unsafe computation.
allow_unsafe is a permission flag, not an assertion; it should not
mark a non-unsafe actor as unsafe. Updated docstring to clarify
the semantic difference between unsafe and allow_unsafe.
- MIN-1: Changed provider/model alias resolution in _extract_v2_actor()
from or-based to is-not-None checks for consistency with
_resolve_actors_map() and add().
- MIN-2: Added test scenario for allow_unsafe=True on non-unsafe YAML.
- MIN-3: Added test scenarios for top-level unsafe: 1 (integer)
rejection without flag and acceptance with flag.
- MIN-4: Added test scenario for graph descriptor additional keys loop
(routes key propagation).
- MIN-5: Added test scenario for non-dict top-level options with nested
options.
- MIN-9: Applied ruff format to step file (removed unnecessary parens).
- NIT-1: Differentiated redundant scenario to test graph descriptor
agent key value instead of duplicating assertions.
- NIT-2: Added test scenario verifying config_blob source: "yaml" default.
- NIT-5: Changed _resolve_actors_map() return type to use
Literal["actors", "agents"] for stronger type safety.
Includes 78 Behave scenarios covering spec-compliant actors: map,
legacy agents: map, top-level fields, rejection of missing provider/model,
combined actor field edge cases, update=True path, schema_version and
compiled_metadata forwarding, actors-as-list edge case, empty actors dict
blocking agents fallback, malformed actor field parts, reverse precedence
for the combined actor field, _extract_v2_options edge cases, unsafe=True
detection, missing name rejection, top-level unsafe, multi-actor
rejection, unsafe coercion edge cases (1.0, 2, 0, "yes", "no"),
top-level graph_descriptor, provider_type/model_id aliases,
_extract_v2_options structural parity with _extract_v2_actor,
allow_unsafe on non-unsafe YAML, top-level unsafe: 1 (integer),
graph descriptor additional keys, non-dict top-level options,
and config_blob source default.
ISSUES CLOSED: #4466