fix(actor): resolve registry.add() rejection of spec-compliant actor YAML
CI / benchmark-regression (push) Has been skipped
CI / helm (push) Successful in 29s
CI / build (push) Successful in 50s
CI / lint (push) Successful in 1m3s
CI / typecheck (push) Successful in 1m23s
CI / quality (push) Successful in 1m38s
CI / security (push) Successful in 1m43s
CI / push-validation (push) Successful in 27s
CI / integration_tests (push) Successful in 3m36s
CI / e2e_tests (push) Successful in 4m6s
CI / unit_tests (push) Successful in 4m53s
CI / docker (push) Successful in 1m29s
CI / coverage (push) Successful in 11m4s
CI / status-check (push) Successful in 4s
CI / benchmark-publish (push) Successful in 1h23m24s
CI / lint (pull_request) Successful in 1m10s
CI / typecheck (pull_request) Successful in 1m16s
CI / security (pull_request) Successful in 1m15s
CI / benchmark-publish (pull_request) Has been skipped
CI / push-validation (pull_request) Successful in 36s
CI / helm (pull_request) Successful in 39s
CI / build (pull_request) Successful in 51s
CI / quality (pull_request) Successful in 1m37s
CI / integration_tests (pull_request) Successful in 3m57s
CI / e2e_tests (pull_request) Successful in 4m13s
CI / unit_tests (pull_request) Successful in 4m27s
CI / docker (pull_request) Successful in 1m30s
CI / coverage (pull_request) Successful in 10m39s
CI / status-check (pull_request) Successful in 3s
CI / benchmark-regression (pull_request) Successful in 1h4m37s
CI / benchmark-regression (push) Has been skipped
CI / helm (push) Successful in 29s
CI / build (push) Successful in 50s
CI / lint (push) Successful in 1m3s
CI / typecheck (push) Successful in 1m23s
CI / quality (push) Successful in 1m38s
CI / security (push) Successful in 1m43s
CI / push-validation (push) Successful in 27s
CI / integration_tests (push) Successful in 3m36s
CI / e2e_tests (push) Successful in 4m6s
CI / unit_tests (push) Successful in 4m53s
CI / docker (push) Successful in 1m29s
CI / coverage (push) Successful in 11m4s
CI / status-check (push) Successful in 4s
CI / benchmark-publish (push) Successful in 1h23m24s
CI / lint (pull_request) Successful in 1m10s
CI / typecheck (pull_request) Successful in 1m16s
CI / security (pull_request) Successful in 1m15s
CI / benchmark-publish (pull_request) Has been skipped
CI / push-validation (pull_request) Successful in 36s
CI / helm (pull_request) Successful in 39s
CI / build (pull_request) Successful in 51s
CI / quality (pull_request) Successful in 1m37s
CI / integration_tests (pull_request) Successful in 3m57s
CI / e2e_tests (pull_request) Successful in 4m13s
CI / unit_tests (pull_request) Successful in 4m27s
CI / docker (pull_request) Successful in 1m30s
CI / coverage (pull_request) Successful in 10m39s
CI / status-check (pull_request) Successful in 3s
CI / benchmark-regression (pull_request) Successful in 1h4m37s
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
This commit was merged in pull request #10796.
This commit is contained in:
@@ -92,9 +92,28 @@ def _empty_summary() -> Summary:
|
||||
"scenarios": {"passed": 0, "failed": 0, "errors": 0, "skipped": 0},
|
||||
"steps": {"passed": 0, "failed": 0, "errors": 0, "skipped": 0},
|
||||
"duration": 0.0,
|
||||
# Each entry is a string in behave's standard format:
|
||||
# " features/path.feature:LINE Scenario name"
|
||||
"failing_scenarios": [],
|
||||
"errored_scenarios": [],
|
||||
}
|
||||
|
||||
|
||||
def _scenario_ref(scenario: Any) -> str:
|
||||
"""Return the canonical behave reference string for a scenario.
|
||||
|
||||
Produces `` features/foo.feature:42 Scenario name`` — the same
|
||||
format that behave itself prints in its "Failing scenarios:" block,
|
||||
so that the ``rui-find-failing-unit-tests`` skill can grep for it
|
||||
directly in CI logs.
|
||||
"""
|
||||
filename = getattr(scenario, "filename", None) or getattr(
|
||||
scenario.feature, "filename", "unknown"
|
||||
)
|
||||
line = getattr(scenario, "line", 0)
|
||||
return f" {filename}:{line} {scenario.name}"
|
||||
|
||||
|
||||
def _extract_summary(runner: Any) -> Summary:
|
||||
"""Build a summary dict from a completed behave Runner."""
|
||||
summary = _empty_summary()
|
||||
@@ -117,8 +136,10 @@ def _extract_summary(runner: Any) -> Summary:
|
||||
summary["scenarios"]["skipped"] += 1
|
||||
elif sname == "failed":
|
||||
summary["scenarios"]["failed"] += 1
|
||||
summary["failing_scenarios"].append(_scenario_ref(scenario))
|
||||
else:
|
||||
summary["scenarios"]["errors"] += 1
|
||||
summary["errored_scenarios"].append(_scenario_ref(scenario))
|
||||
|
||||
for step in scenario.steps:
|
||||
stname = step.status.name
|
||||
@@ -140,6 +161,8 @@ def _merge_summaries(summaries: list[Summary]) -> Summary:
|
||||
for field in ("passed", "failed", "errors", "skipped"):
|
||||
total[bucket][field] += s.get(bucket, {}).get(field, 0)
|
||||
total["duration"] += float(s.get("duration", 0.0))
|
||||
total["failing_scenarios"].extend(s.get("failing_scenarios", []))
|
||||
total["errored_scenarios"].extend(s.get("errored_scenarios", []))
|
||||
return total
|
||||
|
||||
|
||||
@@ -167,6 +190,20 @@ def _print_overall_summary(
|
||||
if wall_seconds is not None:
|
||||
print(f"Wall time: {_format_duration(wall_seconds)}")
|
||||
|
||||
# Print consolidated failing/errored scenario lists in behave's standard
|
||||
# format so CI logs are grep-able with the rui-find-failing-unit-tests
|
||||
# skill. Deduplicate in case multiple workers reported the same scenario.
|
||||
failing = sorted(set(total.get("failing_scenarios", [])))
|
||||
errored = sorted(set(total.get("errored_scenarios", [])))
|
||||
if failing:
|
||||
print("\nFailing scenarios:")
|
||||
for ref in failing:
|
||||
print(ref)
|
||||
if errored:
|
||||
print("\nErrored scenarios:")
|
||||
for ref in errored:
|
||||
print(ref)
|
||||
|
||||
|
||||
def _has_failures(total: Summary) -> bool:
|
||||
return (
|
||||
|
||||
Reference in New Issue
Block a user