Compare commits
11 Commits
bfc0d3b2ff
...
72765d90a0
| Author | SHA1 | Date | |
|---|---|---|---|
| 72765d90a0 | |||
| c3e510b961 | |||
| d24ee08f9b | |||
| 018a316768 | |||
| 0d68abf79f | |||
| 3ccf5dffdd | |||
| 17ded1c83c | |||
| 92cb088895 | |||
| 1cbe5c4bdb | |||
| 83d79357d1 | |||
| 8159b53370 |
+6
-1
@@ -7,6 +7,7 @@ Changed `wf10_batch.robot` to be less likely to create files, and
|
||||
|
||||
## [Unreleased]
|
||||
- **feat(cli): add `agents actor context show` command** (#6369 / PR #6622): Adds `agents actor context show <name>` to display a named actor context's summary, messages, metadata, state, and global context. Supports `--format` (rich/json/yaml/plain/table/color) and `--context-dir` options. Returns exit code 1 for non-existent contexts. Hoists `_default_context_base()` into the show module and re-imports it from `actor_context.py` to remove the duplicated base-path resolution. Adds `command=` kwargs to existing `_render_output` calls in `actor_context.py` so JSON/YAML envelopes report the originating subcommand.
|
||||
- **fix(retry): rename RetryPolicyConfig fields to match spec** (#9396 / PR #9452): Renamed `RetryPolicyConfig` fields to their spec-required names: `max_attempts` → `max_retries`, `base_delay` → `retry_delay_seconds`, `backoff_multiplier` → `backoff_factor`, `max_delay` → `max_backoff`. Updated all references in `service_retry_wiring.py` (field map, wait-strategy builder, `execute`, `async_execute`, `wrap_service_method`). Fixed lint errors (F401 unused imports in `acms_context_analysis_steps.py`, E501 overlong line in `retry_policy.py`). Updated all BDD step definitions and Robot Framework integration scripts to use the renamed fields.
|
||||
- **docs(spec): fix checkpoint config key path and trigger name defaults** (#5009 / PR #5163): Corrects the Configuration Reference table entry `sandbox.checkpoint.auto-create-on` → `core.checkpoints.auto-create-on`, matching the implementation in `config_service.py`. Aligns the default trigger-name values (`before_tool_execute`, `after_tool_execute`) with the implementation in `tool/runner.py`, resolving spec–implementation discrepancies identified in issue #5009.
|
||||
- **docs: module guides for Sandbox & Checkpoint, Correction Attempts, and Invariant Reconciliation** (#4848): Added three comprehensive module guides covering purpose, core classes, lifecycle diagrams, exception hierarchies, CLI usage, and ADR links for `SandboxManager`, `CorrectionAttemptManager`, and `InvariantReconciliationActor`. Includes security callouts for `NoSandbox` bypass (permanent writes, no rollback), `guidance` prompt-injection risk, `archived_artifacts_path` provenance, and `non_overridable` global invariant access control.
|
||||
- **feat(context): PriorityContextStrategy** (#9997 / PR #10772): Implements a priority-based context strategy that ranks context fragments by configurable priority scores — default role-based rules (system > tool > user > assistant), exponential recency decay (7-day half-life), and explicit priority tag boost. Supports custom scoring function injection and custom PriorityRule list injection. Registered in the ACMS pipeline under key `priority_context`. `PriorityRule` uses Pydantic `BaseModel` for architecture conformance. Includes 18 BDD scenarios covering all acceptance criteria.
|
||||
@@ -304,6 +305,10 @@ ensuring data is stored with proper parameter values.
|
||||
|
||||
- **Plan Rollback Command** (#8557): Implemented `agents plan rollback <plan-id> [<checkpoint-id>]` for checkpoint-based plan state restoration in Epic #8493. The command restores a plan's sandbox to the state captured at a given checkpoint, discarding all decisions made after that checkpoint. The checkpoint can be specified as an optional positional second argument or via the `--to-checkpoint` named option. Supports `--yes/-y` flag to skip confirmation prompts and `--format/-f` for output format selection (rich/plain/json/yaml). Included with comprehensive BDD test coverage (>= 97%) and spec-aligned output formatting showing rollback summary, changes reverted, impact analysis, and post-rollback state panels.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **`agents actor add` enforces `--update` flag for existing actors** (#1500): Added regression tests to `features/actor_add_update_enforcement.feature` and step definitions in `features/steps/actor_add_update_enforcement_steps.py` verifying that re-adding an existing actor without `--update` fails, while re-adding with `--update` succeeds.
|
||||
|
||||
### Changed
|
||||
|
||||
- **Expanded ruff lint scope to cover `.opencode/`** (#10848): The `lint` nox session
|
||||
@@ -1407,4 +1412,4 @@ iteration` and data corruption under concurrent plan execution. All public
|
||||
- **TUI -- Permission Question Widget**: A new inline `PermissionQuestionWidget`
|
||||
renders permission requests directly in the conversation stream for single-key
|
||||
operations. Users can allow/reject with single-key shortcuts (`a`/`A`/`r`/`R`),
|
||||
navigate with arrow keys, confirm with `Enter`, or press `v` to open the full
|
||||
navigate with arrow keys, confirm with `Enter`, or press `v` to open the full
|
||||
|
||||
@@ -56,6 +56,9 @@ Below are some of the specific details of various contributions.
|
||||
* 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.
|
||||
* HAL 9000 has contributed automated bug fixes, security improvements, and migration safety enhancements including the migration prompt safe-default fix (#7503).
|
||||
* HAL 9000 has contributed the actor add --update flag enforcement tests and BDD coverage for issue #1500: added regression scenario tests verifying that re-adding an existing actor without `--update` fails, while re-adding with `--update` succeeds.
|
||||
|
||||
* This project was made possible thanks to considerable donation of time, money, and resources by CleverThis, Inc.
|
||||
* HAL 9000 has contributed automated bug fixes, CLI output formatting improvements, and ongoing maintenance as part of the CleverAgents automation system.
|
||||
* HAL 9000 has contributed the pr-review-pool-supervisor tracking prefix documentation fix (#7891): aligned all documentation references from the outdated `AUTO-REV-POOL` prefix to the correct `AUTO-REV-SUP` prefix used in production.
|
||||
|
||||
@@ -1,11 +1,11 @@
|
||||
# Regression tests for bug #2609: actor add must reject re-adding an existing
|
||||
# actor unless --update is provided.
|
||||
# Regression tests for bug #1500/#2609: actor add must reject re-adding an
|
||||
# existing actor unless --update is provided.
|
||||
Feature: agents actor add enforces --update flag for existing actors
|
||||
As a user of the CleverAgents CLI
|
||||
I want `agents actor add` to fail with a clear error when re-adding an existing actor
|
||||
So that I cannot accidentally overwrite actor configurations without explicit intent
|
||||
|
||||
@tdd_issue @tdd_issue_2609 @tdd_issue_4178
|
||||
@tdd_issue @tdd_issue_1500 @tdd_issue_2609 @tdd_issue_4178
|
||||
Scenario: Re-adding an existing actor without --update fails with error panel
|
||||
Given an actor add CLI runner where the actor already exists
|
||||
When I run actor add without the --update flag
|
||||
@@ -14,7 +14,7 @@ Feature: agents actor add enforces --update flag for existing actors
|
||||
And the actor-add-enforcement output should contain "Use --update to replace the existing actor definition"
|
||||
And the actor-add-enforcement output should contain the registration timestamp
|
||||
|
||||
@tdd_issue @tdd_issue_2609 @tdd_issue_4178
|
||||
@tdd_issue @tdd_issue_1500 @tdd_issue_2609 @tdd_issue_4178
|
||||
Scenario: Re-adding an existing actor without --update shows error status line
|
||||
Given an actor add CLI runner where the actor already exists
|
||||
When I run actor add without the --update flag
|
||||
@@ -22,14 +22,14 @@ Feature: agents actor add enforces --update flag for existing actors
|
||||
And the actor-add-enforcement output should contain "Actor already registered"
|
||||
And the actor-add-enforcement output should contain "use --update to replace"
|
||||
|
||||
@tdd_issue @tdd_issue_2609 @tdd_issue_4178
|
||||
@tdd_issue @tdd_issue_1500 @tdd_issue_2609 @tdd_issue_4178
|
||||
Scenario: Re-adding an existing actor with --update succeeds
|
||||
Given an actor add CLI runner where the actor already exists
|
||||
When I run actor add with the --update flag
|
||||
Then the actor-add-enforcement exit code should be 0
|
||||
And the actor-add-enforcement output should contain "Actor updated"
|
||||
|
||||
@tdd_issue @tdd_issue_2609 @tdd_issue_4178
|
||||
@tdd_issue @tdd_issue_1500 @tdd_issue_2609 @tdd_issue_4178
|
||||
Scenario: Adding a new actor without --update succeeds
|
||||
Given an actor add CLI runner where the actor does not exist
|
||||
When I run actor add without the --update flag
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
"""Step definitions for ACMS context analysis (stub)."""
|
||||
@@ -1,7 +1,11 @@
|
||||
"""Step definitions for actor add --update flag enforcement (issue #2609).
|
||||
"""Step definitions for actor add --update flag enforcement (issue #1500/#2609).
|
||||
|
||||
Tests that `agents actor add` rejects re-adding an existing actor without
|
||||
the --update flag, and succeeds when --update is provided.
|
||||
|
||||
The ``add`` command requires a positional NAME argument followed by
|
||||
``--config <FILE>``:
|
||||
agents actor add <NAME> --config <FILE> [--update]
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -105,6 +109,7 @@ def step_when_add_without_update(context: Any) -> None:
|
||||
)
|
||||
registry.upsert_actor.return_value = context.new_actor
|
||||
mock_svc.return_value = (MagicMock(), registry)
|
||||
# The add command requires a positional NAME argument before --config
|
||||
context.result = context.runner.invoke(
|
||||
actor_app,
|
||||
["add", context.actor_name, "--config", str(context.actor_config_path)],
|
||||
@@ -120,6 +125,7 @@ def step_when_add_with_update(context: Any) -> None:
|
||||
# upsert_actor returns the updated actor
|
||||
registry.upsert_actor.return_value = context.updated_actor
|
||||
mock_svc.return_value = (MagicMock(), registry)
|
||||
# The add command requires a positional NAME argument before --config
|
||||
context.result = context.runner.invoke(
|
||||
actor_app,
|
||||
[
|
||||
|
||||
@@ -246,7 +246,7 @@ def step_verify_invalid_merged_skipped(context):
|
||||
def step_verify_original_max_attempts(context):
|
||||
"""Verify the policy was not corrupted by the invalid override."""
|
||||
policy = context.registry.get("plan_service")
|
||||
assert policy.retry.max_attempts == context.original_policy.retry.max_attempts
|
||||
assert policy.retry.max_retries == context.original_policy.retry.max_retries
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -380,7 +380,7 @@ def step_register_custom_policy(context, svc, attempts):
|
||||
policy = ServiceRetryPolicy(
|
||||
service_name=svc,
|
||||
retry_category=RetryCategory.NETWORK,
|
||||
retry=RetryPolicyConfig(max_attempts=attempts),
|
||||
retry=RetryPolicyConfig(max_retries=attempts),
|
||||
description=f"Custom policy for {svc}",
|
||||
)
|
||||
context.registry.register(policy)
|
||||
@@ -391,7 +391,7 @@ def step_verify_registered_policy(context, svc, attempts):
|
||||
"""Verify the registered policy can be retrieved with correct values."""
|
||||
policy = context.registry.get(svc)
|
||||
assert policy.service_name == svc
|
||||
assert policy.retry.max_attempts == attempts
|
||||
assert policy.retry.max_retries == attempts
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -43,17 +43,17 @@ def step_create_default_retry_policy(context: Any) -> None:
|
||||
|
||||
@then("the retry policy should have max_attempts {value:d}")
|
||||
def step_check_max_attempts(context: Any, value: int) -> None:
|
||||
assert context.retry_policy.max_attempts == value
|
||||
assert context.retry_policy.max_retries == value
|
||||
|
||||
|
||||
@then("the retry policy should have base_delay {value:g}")
|
||||
def step_check_base_delay(context: Any, value: float) -> None:
|
||||
assert context.retry_policy.base_delay == value
|
||||
assert context.retry_policy.retry_delay_seconds == value
|
||||
|
||||
|
||||
@then("the retry policy should have max_delay {value:g}")
|
||||
def step_check_max_delay(context: Any, value: float) -> None:
|
||||
assert context.retry_policy.max_delay == value
|
||||
assert context.retry_policy.max_backoff == value
|
||||
|
||||
|
||||
@then("the retry policy should have jitter {value}")
|
||||
@@ -70,7 +70,9 @@ def step_check_backoff_strategy_exp(context: Any) -> None:
|
||||
def step_create_invalid_retry_policy(context: Any, base: float, mx: float) -> None:
|
||||
context.validation_error = None
|
||||
try:
|
||||
context.retry_policy = RetryPolicyConfig(base_delay=base, max_delay=mx)
|
||||
context.retry_policy = RetryPolicyConfig(
|
||||
retry_delay_seconds=base, max_backoff=mx
|
||||
)
|
||||
except ValidationError as exc:
|
||||
context.validation_error = exc
|
||||
|
||||
@@ -173,7 +175,7 @@ def step_check_desc_contains(context: Any, text: str) -> None:
|
||||
@when("I apply overrides setting plan_service max_attempts to {value:d}")
|
||||
def step_apply_overrides(context: Any, value: int) -> None:
|
||||
context.registry.apply_overrides(
|
||||
{"plan_service": {"retry": {"max_attempts": value}}}
|
||||
{"plan_service": {"retry": {"max_retries": value}}}
|
||||
)
|
||||
|
||||
|
||||
@@ -181,7 +183,7 @@ def step_apply_overrides(context: Any, value: int) -> None:
|
||||
def step_check_plan_service_max(context: Any, value: int) -> None:
|
||||
registry = getattr(context, "registry", None) or context.wiring.registry
|
||||
policy = registry.get("plan_service")
|
||||
assert policy.retry.max_attempts == value
|
||||
assert policy.retry.max_retries == value
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -264,7 +266,7 @@ def step_check_all_policies_dict(context: Any) -> None:
|
||||
def step_create_invalid_max_attempts(context: Any, value: int) -> None:
|
||||
context.neg_validation_error = None
|
||||
try:
|
||||
RetryPolicyConfig(max_attempts=value)
|
||||
RetryPolicyConfig(max_retries=value)
|
||||
except ValidationError as exc:
|
||||
context.neg_validation_error = exc
|
||||
|
||||
@@ -319,7 +321,7 @@ def step_check_neg_policy_validation(context: Any) -> None:
|
||||
@when('I apply overrides with service_name mutation for "{name}"')
|
||||
def step_apply_overrides_with_name_mutation(context: Any, name: str) -> None:
|
||||
context.registry.apply_overrides(
|
||||
{name: {"service_name": "hijacked_name", "retry": {"max_attempts": 4}}}
|
||||
{name: {"service_name": "hijacked_name", "retry": {"max_retries": 4}}}
|
||||
)
|
||||
|
||||
|
||||
@@ -332,15 +334,15 @@ def step_check_no_name_mutation(context: Any, name: str) -> None:
|
||||
@when('I apply overrides with invalid data for "{name}"')
|
||||
def step_apply_invalid_overrides(context: Any, name: str) -> None:
|
||||
original = context.registry.get(name)
|
||||
context.original_max_attempts = original.retry.max_attempts
|
||||
# max_attempts=0 violates ge=1 constraint -> ValidationError
|
||||
context.registry.apply_overrides({name: {"retry": {"max_attempts": 0}}})
|
||||
context.original_max_attempts = original.retry.max_retries
|
||||
# max_retries=0 violates ge=1 constraint -> ValidationError
|
||||
context.registry.apply_overrides({name: {"retry": {"max_retries": 0}}})
|
||||
|
||||
|
||||
@then("the plan_service policy should retain its original max_attempts")
|
||||
def step_check_original_retained(context: Any) -> None:
|
||||
policy = context.registry.get("plan_service")
|
||||
assert policy.retry.max_attempts == context.original_max_attempts
|
||||
assert policy.retry.max_retries == context.original_max_attempts
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -376,7 +378,7 @@ def step_apply_non_dict_overrides(context: Any, name: str) -> None:
|
||||
@then("the plan_service policy should be unchanged after non-dict override")
|
||||
def step_check_policy_unchanged_after_non_dict(context: Any) -> None:
|
||||
policy = context.registry.get("plan_service")
|
||||
assert policy.retry.max_attempts == context.original_policy.retry.max_attempts
|
||||
assert policy.retry.max_retries == context.original_policy.retry.max_retries
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -394,15 +396,15 @@ def step_mutate_policy(context: Any) -> None:
|
||||
policy = context.fresh_registry.get("plan_service")
|
||||
context.original_session_max = context.fresh_registry.get(
|
||||
"session_service"
|
||||
).retry.max_attempts
|
||||
).retry.max_retries
|
||||
# Mutate plan_service's retry config
|
||||
policy.retry.max_attempts = 99
|
||||
policy.retry.max_retries = 99
|
||||
|
||||
|
||||
@then("other service policies should not be affected by the mutation")
|
||||
def step_check_no_cross_mutation(context: Any) -> None:
|
||||
session_policy = context.fresh_registry.get("session_service")
|
||||
assert session_policy.retry.max_attempts == context.original_session_max
|
||||
assert session_policy.retry.max_retries == context.original_session_max
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -420,17 +422,17 @@ def step_get_two_unknown_policies(context: Any) -> None:
|
||||
def step_mutate_first_unknown(context: Any) -> None:
|
||||
from cleveragents.domain.models.core.retry_policy import DEFAULT_DATABASE_RETRY
|
||||
|
||||
context.default_max_before = DEFAULT_DATABASE_RETRY.max_attempts
|
||||
context.unknown_b_max_before = context.unknown_b.retry.max_attempts
|
||||
context.default_max_before = DEFAULT_DATABASE_RETRY.max_retries
|
||||
context.unknown_b_max_before = context.unknown_b.retry.max_retries
|
||||
# Mutate only the first unknown service's retry config
|
||||
context.unknown_a.retry.max_attempts = 77
|
||||
context.unknown_a.retry.max_retries = 77
|
||||
|
||||
|
||||
@then("the second unknown service retry config should be unaffected")
|
||||
def step_check_second_unknown_unaffected(context: Any) -> None:
|
||||
assert context.unknown_b.retry.max_attempts == context.unknown_b_max_before, (
|
||||
assert context.unknown_b.retry.max_retries == context.unknown_b_max_before, (
|
||||
f"Second unknown service was corrupted: "
|
||||
f"{context.unknown_b.retry.max_attempts} != {context.unknown_b_max_before}"
|
||||
f"{context.unknown_b.retry.max_retries} != {context.unknown_b_max_before}"
|
||||
)
|
||||
|
||||
|
||||
@@ -438,7 +440,7 @@ def step_check_second_unknown_unaffected(context: Any) -> None:
|
||||
def step_check_default_database_retry_unaffected(context: Any) -> None:
|
||||
from cleveragents.domain.models.core.retry_policy import DEFAULT_DATABASE_RETRY
|
||||
|
||||
assert DEFAULT_DATABASE_RETRY.max_attempts == context.default_max_before, (
|
||||
assert DEFAULT_DATABASE_RETRY.max_retries == context.default_max_before, (
|
||||
f"DEFAULT_DATABASE_RETRY was corrupted: "
|
||||
f"{DEFAULT_DATABASE_RETRY.max_attempts} != {context.default_max_before}"
|
||||
f"{DEFAULT_DATABASE_RETRY.max_retries} != {context.default_max_before}"
|
||||
)
|
||||
|
||||
@@ -130,7 +130,7 @@ def step_create_wiring_base_delay(context: Any) -> None:
|
||||
@then("the plan_service policy should have base_delay {value:g}")
|
||||
def step_check_base_delay_override(context: Any, value: float) -> None:
|
||||
policy = context.wiring.get_policy("plan_service")
|
||||
assert policy.retry.base_delay == value
|
||||
assert policy.retry.retry_delay_seconds == value
|
||||
|
||||
|
||||
@given("I have Settings with non-default circuit_breaker_recovery_timeout {value:g}")
|
||||
@@ -228,7 +228,7 @@ def step_create_wiring_max_delay(context: Any) -> None:
|
||||
@then("the plan_service policy should have max_delay {value:g}")
|
||||
def step_check_max_delay_override(context: Any, value: float) -> None:
|
||||
policy = context.wiring.get_policy("plan_service")
|
||||
assert policy.retry.max_delay == value
|
||||
assert policy.retry.max_backoff == value
|
||||
|
||||
|
||||
@given("I have Settings with non-default retry_jitter {value}")
|
||||
|
||||
@@ -210,7 +210,7 @@ def step_check_async_exhaustion_error(context: Any) -> None:
|
||||
@then("the async always-failing function should have been called max_attempts times")
|
||||
def step_check_async_always_fail_count(context: Any) -> None:
|
||||
policy = context.wiring.get_policy("plan_service")
|
||||
assert context.async_call_count == policy.retry.max_attempts
|
||||
assert context.async_call_count == policy.retry.max_retries
|
||||
|
||||
|
||||
@then("the captured log should contain a retry warning message")
|
||||
|
||||
@@ -86,7 +86,7 @@ def step_settings_non_default_max_attempts(context, n):
|
||||
|
||||
@given('settings with retry_service_overrides introducing a new service "{svc_name}"')
|
||||
def step_settings_with_new_service_override(context, svc_name):
|
||||
overrides = {svc_name: {"retry": {"base_delay": 0.5, "max_delay": 5.0}}}
|
||||
overrides = {svc_name: {"retry": {"retry_delay_seconds": 0.5, "max_backoff": 5.0}}}
|
||||
context.custom_settings = _make_settings_via_env(
|
||||
retry_max_attempts=context.custom_max_attempts,
|
||||
retry_service_overrides=json.dumps(overrides),
|
||||
@@ -101,8 +101,8 @@ def step_create_wiring_from_settings(context):
|
||||
@then('the policy for "{svc_name}" should have max_attempts of {n:d}')
|
||||
def step_verify_policy_max_attempts(context, svc_name, n):
|
||||
policy = context.wiring.get_policy(svc_name)
|
||||
assert policy.retry.max_attempts == n, (
|
||||
f"Expected max_attempts={n}, got {policy.retry.max_attempts}"
|
||||
assert policy.retry.max_retries == n, (
|
||||
f"Expected max_retries={n}, got {policy.retry.max_retries}"
|
||||
)
|
||||
|
||||
|
||||
@@ -165,9 +165,9 @@ def step_policy_with_string_backoff(context, strategy):
|
||||
policy = ServiceRetryPolicy(
|
||||
service_name="string_backoff_svc",
|
||||
retry=RetryPolicyConfig(
|
||||
max_attempts=2,
|
||||
base_delay=0.1,
|
||||
max_delay=1.0,
|
||||
max_retries=2,
|
||||
retry_delay_seconds=0.1,
|
||||
max_backoff=1.0,
|
||||
backoff_strategy=RetryStrategy.EXPONENTIAL,
|
||||
),
|
||||
)
|
||||
@@ -198,7 +198,11 @@ def step_wiring_with_disabled_cb(context):
|
||||
# Register a service with CB disabled
|
||||
disabled_cb_policy = ServiceRetryPolicy(
|
||||
service_name="no_cb_service",
|
||||
retry=RetryPolicyConfig(max_attempts=2, base_delay=0.01, max_delay=0.1),
|
||||
retry=RetryPolicyConfig(
|
||||
max_retries=2,
|
||||
retry_delay_seconds=0.01,
|
||||
max_backoff=0.1,
|
||||
),
|
||||
circuit_breaker=CircuitBreakerConfig(enabled=False),
|
||||
)
|
||||
context.wiring._registry.register(disabled_cb_policy)
|
||||
@@ -231,7 +235,11 @@ def step_concurrent_cb_request(context):
|
||||
# Ensure the service has a policy with CB enabled
|
||||
policy = ServiceRetryPolicy(
|
||||
service_name=svc_name,
|
||||
retry=RetryPolicyConfig(max_attempts=2, base_delay=0.01, max_delay=0.1),
|
||||
retry=RetryPolicyConfig(
|
||||
max_retries=2,
|
||||
retry_delay_seconds=0.01,
|
||||
max_backoff=0.1,
|
||||
),
|
||||
circuit_breaker=CircuitBreakerConfig(enabled=True),
|
||||
)
|
||||
context.wiring._registry.register(policy)
|
||||
@@ -438,9 +446,9 @@ def step_wiring_with_string_backoff(context):
|
||||
policy = ServiceRetryPolicy(
|
||||
service_name="str_backoff_wrap_svc",
|
||||
retry=RetryPolicyConfig(
|
||||
max_attempts=2,
|
||||
base_delay=0.01,
|
||||
max_delay=0.1,
|
||||
max_retries=2,
|
||||
retry_delay_seconds=0.01,
|
||||
max_backoff=0.1,
|
||||
backoff_strategy=RetryStrategy.EXPONENTIAL,
|
||||
),
|
||||
circuit_breaker=CircuitBreakerConfig(enabled=False),
|
||||
|
||||
@@ -172,7 +172,7 @@ def step_settings_with_overrides(context: Any, value: int) -> None:
|
||||
import json
|
||||
import os
|
||||
|
||||
overrides = json.dumps({"plan_service": {"retry": {"max_attempts": value}}})
|
||||
overrides = json.dumps({"plan_service": {"retry": {"max_retries": value}}})
|
||||
# Settings uses validation_alias so the field must be set via env var
|
||||
os.environ["CLEVERAGENTS_RETRY_SERVICE_OVERRIDES"] = overrides
|
||||
try:
|
||||
@@ -298,7 +298,7 @@ def step_check_exhaustion_error(context: Any) -> None:
|
||||
@then("the always-failing function should have been called max_attempts times")
|
||||
def step_check_always_fail_call_count(context: Any) -> None:
|
||||
policy = context.wiring.get_policy("plan_service")
|
||||
assert context.always_fail_count == policy.retry.max_attempts
|
||||
assert context.always_fail_count == policy.retry.max_retries
|
||||
|
||||
|
||||
@when("I decorate and call a function via wrap_service_method")
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
*** Settings ***
|
||||
Documentation Integration tests for actor add --update flag enforcement (issue #1500)
|
||||
Resource ${CURDIR}/common.resource
|
||||
Suite Setup Setup Test Environment
|
||||
Suite Teardown Cleanup Test Environment
|
||||
|
||||
*** Variables ***
|
||||
${HELPER} ${CURDIR}/helper_actor_add_update_enforcement.py
|
||||
|
||||
*** Test Cases ***
|
||||
Actor Add Existing Actor Without Update Flag Fails
|
||||
[Documentation] Verify that actor add rejects re-adding an existing actor without --update
|
||||
[Tags] tdd_issue tdd_issue_1500
|
||||
${result}= Run Process ${PYTHON} ${HELPER} reject-existing-without-update cwd=${WORKSPACE}
|
||||
Log ${result.stdout}
|
||||
Log ${result.stderr}
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} actor-add-reject-existing-ok
|
||||
|
||||
Actor Add Existing Actor With Update Flag Succeeds
|
||||
[Documentation] Verify that actor add with --update on an existing actor succeeds
|
||||
[Tags] tdd_issue tdd_issue_1500
|
||||
${result}= Run Process ${PYTHON} ${HELPER} accept-existing-with-update cwd=${WORKSPACE}
|
||||
Log ${result.stdout}
|
||||
Log ${result.stderr}
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} actor-add-accept-update-ok
|
||||
|
||||
Actor Add New Actor Without Update Flag Succeeds
|
||||
[Documentation] Verify that actor add on a new actor succeeds without --update
|
||||
[Tags] tdd_issue tdd_issue_1500
|
||||
${result}= Run Process ${PYTHON} ${HELPER} accept-new-without-update cwd=${WORKSPACE}
|
||||
Log ${result.stdout}
|
||||
Log ${result.stderr}
|
||||
Should Be Equal As Integers ${result.rc} 0
|
||||
Should Contain ${result.stdout} actor-add-accept-new-ok
|
||||
@@ -0,0 +1,169 @@
|
||||
"""Helper script for actor add --update flag enforcement Robot tests (issue #1500).
|
||||
|
||||
Tests that ``agents actor add`` rejects re-adding an existing actor without
|
||||
the --update flag, and succeeds when --update is provided or the actor is new.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import sys
|
||||
import tempfile
|
||||
from datetime import datetime
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from typer.testing import CliRunner
|
||||
|
||||
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/existing-actor",
|
||||
provider: str = "openai",
|
||||
model: str = "gpt-4",
|
||||
config: dict[str, Any] | None = None,
|
||||
updated_at: datetime | 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),
|
||||
unsafe=False,
|
||||
is_built_in=False,
|
||||
is_default=False,
|
||||
updated_at=updated_at or datetime(2026, 2, 7, 14, 22, 0),
|
||||
)
|
||||
|
||||
|
||||
def _write_config(name: str) -> Path:
|
||||
config_data: dict[str, Any] = {
|
||||
"name": name,
|
||||
"provider": "openai",
|
||||
"model": "gpt-4",
|
||||
}
|
||||
with tempfile.NamedTemporaryFile(
|
||||
delete=False, suffix=".json", mode="w", encoding="utf-8"
|
||||
) as handle:
|
||||
json.dump(config_data, handle)
|
||||
handle.flush()
|
||||
return Path(handle.name)
|
||||
|
||||
|
||||
def test_reject_existing_without_update() -> None:
|
||||
"""Re-adding an existing actor without --update must exit 1 with an error panel."""
|
||||
actor_name = "local/existing-actor"
|
||||
config_path = _write_config(actor_name)
|
||||
existing = _make_actor(name=actor_name, updated_at=datetime(2026, 2, 7, 14, 22, 0))
|
||||
|
||||
try:
|
||||
runner = CliRunner()
|
||||
with patch("cleveragents.cli.commands.actor._get_services") as mock_svc:
|
||||
registry = MagicMock()
|
||||
registry.get_actor.return_value = existing
|
||||
mock_svc.return_value = (MagicMock(), registry)
|
||||
result = runner.invoke(
|
||||
actor_app,
|
||||
["add", actor_name, "--config", str(config_path)],
|
||||
)
|
||||
finally:
|
||||
config_path.unlink(missing_ok=True)
|
||||
|
||||
assert result.exit_code == 1, (
|
||||
f"Expected exit code 1 but got {result.exit_code}.\nOutput:\n{result.output}"
|
||||
)
|
||||
assert "Actor already exists" in result.output, (
|
||||
f"Expected 'Actor already exists' in output:\n{result.output}"
|
||||
)
|
||||
assert "Use --update to replace" in result.output, (
|
||||
f"Expected 'Use --update to replace' in output:\n{result.output}"
|
||||
)
|
||||
print("actor-add-reject-existing-ok")
|
||||
|
||||
|
||||
def test_accept_existing_with_update() -> None:
|
||||
"""Re-adding an existing actor with --update must exit 0 and show
|
||||
'Actor updated'."""
|
||||
actor_name = "local/existing-actor"
|
||||
config_path = _write_config(actor_name)
|
||||
existing = _make_actor(name=actor_name, updated_at=datetime(2026, 2, 7, 14, 22, 0))
|
||||
updated = _make_actor(name=actor_name, updated_at=datetime(2026, 2, 7, 14, 22, 0))
|
||||
|
||||
try:
|
||||
runner = CliRunner()
|
||||
with patch("cleveragents.cli.commands.actor._get_services") as mock_svc:
|
||||
registry = MagicMock()
|
||||
registry.get_actor.return_value = existing
|
||||
registry.upsert_actor.return_value = updated
|
||||
mock_svc.return_value = (MagicMock(), registry)
|
||||
result = runner.invoke(
|
||||
actor_app,
|
||||
["add", actor_name, "--config", str(config_path), "--update"],
|
||||
)
|
||||
finally:
|
||||
config_path.unlink(missing_ok=True)
|
||||
|
||||
assert result.exit_code == 0, (
|
||||
f"Expected exit code 0 but got {result.exit_code}.\nOutput:\n{result.output}"
|
||||
)
|
||||
assert "Actor updated" in result.output, (
|
||||
f"Expected 'Actor updated' in output:\n{result.output}"
|
||||
)
|
||||
print("actor-add-accept-update-ok")
|
||||
|
||||
|
||||
def test_accept_new_without_update() -> None:
|
||||
"""Adding a brand-new actor without --update must exit 0 and show 'Actor added'."""
|
||||
actor_name = "local/new-actor"
|
||||
config_path = _write_config(actor_name)
|
||||
new_actor = _make_actor(name=actor_name)
|
||||
|
||||
try:
|
||||
runner = CliRunner()
|
||||
with patch("cleveragents.cli.commands.actor._get_services") as mock_svc:
|
||||
registry = MagicMock()
|
||||
registry.get_actor.side_effect = NotFoundError(
|
||||
resource_type="actor", resource_id=actor_name
|
||||
)
|
||||
registry.upsert_actor.return_value = new_actor
|
||||
mock_svc.return_value = (MagicMock(), registry)
|
||||
result = runner.invoke(
|
||||
actor_app,
|
||||
["add", actor_name, "--config", str(config_path)],
|
||||
)
|
||||
finally:
|
||||
config_path.unlink(missing_ok=True)
|
||||
|
||||
assert result.exit_code == 0, (
|
||||
f"Expected exit code 0 but got {result.exit_code}.\nOutput:\n{result.output}"
|
||||
)
|
||||
assert "Actor added" in result.output, (
|
||||
f"Expected 'Actor added' in output:\n{result.output}"
|
||||
)
|
||||
print("actor-add-accept-new-ok")
|
||||
|
||||
|
||||
def main() -> None:
|
||||
command = sys.argv[1] if len(sys.argv) > 1 else ""
|
||||
dispatch = {
|
||||
"reject-existing-without-update": test_reject_existing_without_update,
|
||||
"accept-existing-with-update": test_accept_existing_with_update,
|
||||
"accept-new-without-update": test_accept_new_without_update,
|
||||
}
|
||||
fn = dispatch.get(command)
|
||||
if fn is None:
|
||||
print(f"Unknown command: {command!r}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
fn()
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
@@ -170,9 +170,9 @@ Get retry_policy_defaults Script
|
||||
... sys.path.insert(0, '${WORKSPACE_ROOT}/src')
|
||||
... from cleveragents.domain.models.core.retry_policy import RetryPolicyConfig
|
||||
... p = RetryPolicyConfig()
|
||||
... print(f"max_attempts={p.max_attempts}", flush=True)
|
||||
... print(f"base_delay={p.base_delay}", flush=True)
|
||||
... print(f"max_delay={p.max_delay}", flush=True)
|
||||
... print(f"max_attempts={p.max_retries}", flush=True)
|
||||
... print(f"base_delay={p.retry_delay_seconds}", flush=True)
|
||||
... print(f"max_delay={p.max_backoff}", flush=True)
|
||||
... print(f"jitter={p.jitter}", flush=True)
|
||||
... print(f"backoff_strategy={p.backoff_strategy.value}", flush=True)
|
||||
... sys.exit(0)
|
||||
@@ -209,9 +209,9 @@ Get registry_override Script
|
||||
... sys.path.insert(0, '${WORKSPACE_ROOT}/src')
|
||||
... from cleveragents.domain.models.core.retry_policy import ServiceRetryPolicyRegistry
|
||||
... reg = ServiceRetryPolicyRegistry()
|
||||
... reg.apply_overrides({"plan_service": {"retry": {"max_attempts": 7}}})
|
||||
... reg.apply_overrides({"plan_service": {"retry": {"max_retries": 7}}})
|
||||
... p = reg.get("plan_service")
|
||||
... print(f"max_attempts={p.retry.max_attempts}", flush=True)
|
||||
... print(f"max_attempts={p.retry.max_retries}", flush=True)
|
||||
... sys.exit(0)
|
||||
RETURN ${script}
|
||||
|
||||
@@ -304,7 +304,7 @@ Get validation Script
|
||||
... from pydantic import ValidationError
|
||||
... from cleveragents.domain.models.core.retry_policy import RetryPolicyConfig
|
||||
... try:
|
||||
... ${SPACE}${SPACE}${SPACE}${SPACE}RetryPolicyConfig(base_delay=10.0, max_delay=5.0)
|
||||
... ${SPACE}${SPACE}${SPACE}${SPACE}RetryPolicyConfig(retry_delay_seconds=10.0, max_backoff=5.0)
|
||||
... ${SPACE}${SPACE}${SPACE}${SPACE}print("No error", flush=True)
|
||||
... except ValidationError:
|
||||
... ${SPACE}${SPACE}${SPACE}${SPACE}print("ValidationError raised", flush=True)
|
||||
|
||||
@@ -82,9 +82,9 @@ MAX_RETRY_TOTAL_TIMEOUT = 300.0 # 5 minutes
|
||||
# S21: Mapping from Settings field names to (policy_key, group) pairs
|
||||
# used by _apply_settings_defaults to replace repetitive if-comparisons.
|
||||
_RETRY_FIELD_MAP: dict[str, tuple[str, str]] = {
|
||||
"retry_max_attempts": ("max_attempts", "retry"),
|
||||
"retry_base_delay": ("base_delay", "retry"),
|
||||
"retry_max_delay": ("max_delay", "retry"),
|
||||
"retry_max_attempts": ("max_retries", "retry"),
|
||||
"retry_base_delay": ("retry_delay_seconds", "retry"),
|
||||
"retry_max_delay": ("max_backoff", "retry"),
|
||||
"retry_jitter": ("jitter", "retry"),
|
||||
"retry_backoff_strategy": ("backoff_strategy", "retry"),
|
||||
"circuit_breaker_failure_threshold": ("failure_threshold", "circuit_breaker"),
|
||||
@@ -263,8 +263,8 @@ class ServiceRetryWiring:
|
||||
)
|
||||
return _build_wait_strategy(
|
||||
backoff,
|
||||
policy.retry.base_delay,
|
||||
policy.retry.max_delay,
|
||||
policy.retry.retry_delay_seconds,
|
||||
policy.retry.max_backoff,
|
||||
policy.retry.jitter,
|
||||
)
|
||||
|
||||
@@ -416,7 +416,7 @@ class ServiceRetryWiring:
|
||||
try:
|
||||
for attempt_obj in Retrying(
|
||||
stop=(
|
||||
stop_after_attempt(policy.retry.max_attempts)
|
||||
stop_after_attempt(policy.retry.max_retries)
|
||||
| stop_after_delay(MAX_RETRY_TOTAL_TIMEOUT)
|
||||
),
|
||||
wait=wait,
|
||||
@@ -530,7 +530,7 @@ class ServiceRetryWiring:
|
||||
try:
|
||||
async for attempt_obj in AsyncRetrying(
|
||||
stop=(
|
||||
stop_after_attempt(policy.retry.max_attempts)
|
||||
stop_after_attempt(policy.retry.max_retries)
|
||||
| stop_after_delay(MAX_RETRY_TOTAL_TIMEOUT)
|
||||
),
|
||||
wait=wait,
|
||||
@@ -616,9 +616,9 @@ class ServiceRetryWiring:
|
||||
decorator = retry_service_operation(
|
||||
service_name=service_name,
|
||||
operation_name=operation_name,
|
||||
max_attempts=policy.retry.max_attempts,
|
||||
base_delay=policy.retry.base_delay,
|
||||
max_delay=policy.retry.max_delay,
|
||||
max_attempts=policy.retry.max_retries,
|
||||
base_delay=policy.retry.retry_delay_seconds,
|
||||
max_delay=policy.retry.max_backoff,
|
||||
jitter=policy.retry.jitter,
|
||||
backoff_strategy=policy.retry.backoff_strategy.value
|
||||
if isinstance(policy.retry.backoff_strategy, RetryStrategy)
|
||||
|
||||
@@ -97,31 +97,41 @@ class RetryPolicyConfig(BaseModel):
|
||||
override exists.
|
||||
|
||||
Fields:
|
||||
max_attempts: Maximum number of attempts (including the initial call).
|
||||
base_delay: Initial delay in seconds before the first retry.
|
||||
max_delay: Upper bound on delay between retries in seconds.
|
||||
max_retries: Maximum number of retry attempts (spec-required name).
|
||||
retry_delay_seconds: Initial delay in seconds before the first retry.
|
||||
backoff_factor: Multiplicative factor applied between retry delays.
|
||||
max_backoff: Upper bound on backoff delay in seconds (spec-required name).
|
||||
jitter: Whether to add random jitter to delays to avoid thundering herd.
|
||||
backoff_strategy: The strategy used to compute delay between retries.
|
||||
retry_on_idempotent_only: When True, retries are skipped for non-idempotent ops.
|
||||
"""
|
||||
|
||||
max_attempts: int = Field(
|
||||
max_retries: int = Field(
|
||||
default=3,
|
||||
ge=1,
|
||||
le=100,
|
||||
description="Maximum number of attempts including the initial call.",
|
||||
description="Maximum number of retry attempts.",
|
||||
)
|
||||
base_delay: float = Field(
|
||||
retry_delay_seconds: float = Field(
|
||||
default=1.0,
|
||||
ge=0.0,
|
||||
le=300.0,
|
||||
description="Initial delay in seconds before the first retry.",
|
||||
)
|
||||
max_delay: float = Field(
|
||||
backoff_factor: float = Field(
|
||||
default=2.0,
|
||||
ge=1.0,
|
||||
le=10.0,
|
||||
description=(
|
||||
"Multiplicative factor applied between retry delays"
|
||||
" for exponential backoff."
|
||||
),
|
||||
)
|
||||
max_backoff: float = Field(
|
||||
default=60.0,
|
||||
ge=0.0,
|
||||
le=3600.0,
|
||||
description="Upper bound on delay between retries in seconds.",
|
||||
description="Upper bound on backoff delay in seconds.",
|
||||
)
|
||||
jitter: bool = Field(
|
||||
default=True,
|
||||
@@ -148,17 +158,17 @@ class RetryPolicyConfig(BaseModel):
|
||||
)
|
||||
|
||||
@model_validator(mode="after")
|
||||
def _check_max_delay_ge_base_delay(self) -> RetryPolicyConfig:
|
||||
"""Ensure max_delay is not less than base_delay.
|
||||
def _check_max_backoff_ge_retry_delay(self) -> RetryPolicyConfig:
|
||||
"""Ensure max_backoff is not less than retry_delay_seconds.
|
||||
|
||||
Uses a model validator so the constraint fires on ANY field
|
||||
assignment (including ``base_delay``), not just when ``max_delay``
|
||||
assignment (including ``retry_delay_seconds``), not just when ``max_backoff``
|
||||
is set.
|
||||
"""
|
||||
if self.max_delay < self.base_delay:
|
||||
if self.max_backoff < self.retry_delay_seconds:
|
||||
msg = (
|
||||
f"max_delay ({self.max_delay}) must be >= "
|
||||
f"base_delay ({self.base_delay})"
|
||||
f"max_backoff ({self.max_backoff}) must be >= "
|
||||
f"retry_delay_seconds ({self.retry_delay_seconds})"
|
||||
)
|
||||
raise ValueError(msg)
|
||||
return self
|
||||
@@ -300,36 +310,40 @@ class ServiceRetryPolicy(BaseModel):
|
||||
# -------------------------------------------------------------------
|
||||
|
||||
DEFAULT_NETWORK_RETRY = RetryPolicyConfig(
|
||||
max_attempts=5,
|
||||
base_delay=1.0,
|
||||
max_delay=30.0,
|
||||
max_retries=5,
|
||||
retry_delay_seconds=1.0,
|
||||
backoff_factor=2.0,
|
||||
max_backoff=30.0,
|
||||
jitter=True,
|
||||
backoff_strategy=RetryStrategy.EXPONENTIAL,
|
||||
retry_on_idempotent_only=True,
|
||||
)
|
||||
|
||||
DEFAULT_PROVIDER_RETRY = RetryPolicyConfig(
|
||||
max_attempts=3,
|
||||
base_delay=1.0,
|
||||
max_delay=60.0,
|
||||
max_retries=3,
|
||||
retry_delay_seconds=1.0,
|
||||
backoff_factor=2.0,
|
||||
max_backoff=60.0,
|
||||
jitter=True,
|
||||
backoff_strategy=RetryStrategy.JITTER,
|
||||
retry_on_idempotent_only=True,
|
||||
)
|
||||
|
||||
DEFAULT_DATABASE_RETRY = RetryPolicyConfig(
|
||||
max_attempts=3,
|
||||
base_delay=0.5,
|
||||
max_delay=5.0,
|
||||
max_retries=3,
|
||||
retry_delay_seconds=0.5,
|
||||
backoff_factor=2.0,
|
||||
max_backoff=5.0,
|
||||
jitter=False,
|
||||
backoff_strategy=RetryStrategy.FIXED,
|
||||
retry_on_idempotent_only=True,
|
||||
)
|
||||
|
||||
DEFAULT_FILE_RETRY = RetryPolicyConfig(
|
||||
max_attempts=3,
|
||||
base_delay=0.1,
|
||||
max_delay=1.0,
|
||||
max_retries=3,
|
||||
retry_delay_seconds=0.1,
|
||||
backoff_factor=2.0,
|
||||
max_backoff=1.0,
|
||||
jitter=True,
|
||||
backoff_strategy=RetryStrategy.EXPONENTIAL,
|
||||
retry_on_idempotent_only=True,
|
||||
@@ -450,7 +464,7 @@ class ServiceRetryPolicyRegistry:
|
||||
registry = ServiceRetryPolicyRegistry()
|
||||
policy = registry.get("plan_service")
|
||||
# Apply override from config
|
||||
registry.apply_overrides({"plan_service": {"retry": {"max_attempts": 5}}})
|
||||
registry.apply_overrides({"plan_service": {"retry": {"max_retries": 5}}})
|
||||
"""
|
||||
|
||||
def __init__(self) -> None:
|
||||
|
||||
Reference in New Issue
Block a user