Files
cleveragents-core/features/steps/retry_review_fixes_steps.py
T
CoreRasurae 4d3499dcfb
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 16s
CI / build (pull_request) Successful in 17s
CI / quality (pull_request) Successful in 18s
CI / security (pull_request) Successful in 35s
CI / typecheck (pull_request) Successful in 38s
CI / unit_tests (pull_request) Successful in 2m43s
CI / integration_tests (pull_request) Successful in 3m20s
CI / docker (pull_request) Successful in 51s
CI / coverage (pull_request) Successful in 6m18s
CI / build (push) Successful in 14s
CI / lint (push) Successful in 21s
CI / quality (push) Successful in 29s
CI / typecheck (push) Successful in 35s
CI / security (push) Successful in 36s
CI / benchmark-regression (push) Has been skipped
CI / unit_tests (push) Successful in 3m17s
CI / docker (push) Successful in 9s
CI / integration_tests (push) Successful in 3m29s
CI / coverage (push) Successful in 7m3s
CI / benchmark-publish (push) Successful in 19m50s
CI / benchmark-regression (pull_request) Successful in 40m6s
feat(async): wire retry policies into services
Wire per-service retry policies and circuit breakers into the service
layer via ServiceRetryWiring, backed by ServiceRetryPolicyRegistry and
configurable through Settings environment variables.

Production hardening from code review:
- Fix TOCTOU race in CircuitBreaker._on_success (half-open state)
- Add half-open probe limit to prevent unbounded concurrent requests
- Track all exception types for circuit breaker failure counting
- Detect async callables wrapped in functools.partial and callable objects
- Enforce spec-compliant 2s minimum for linear backoff strategy
- Enforce 0.1s floor for fixed backoff strategy
- Add retry amplification guard via contextvars nesting depth tracking
- Cap total retry wall-clock time at 300s (MAX_RETRY_TOTAL_TIMEOUT)
- Sanitize exception messages in retry logs to prevent secret leakage
- Fix wrap_service_method TOCTOU by holding cache lock for full operation
- Deep-copy default policies to prevent cross-policy mutation
- Warn on unknown override keys in apply_overrides
- Guard apply_overrides against non-dict and deeply nested JSON values
- Read circuit breaker state under lock in is_circuit_open
- Catch RecursionError in JSON config parsing
- Add total_timeout + nesting guard to retry_service_operation decorator
- Extend secret sanitization to Authorization headers, private_key,
  connection_string, and access_key patterns
- Enforce 0.1s floor on jitter backoff strategy
- Cache wait strategies per service in ServiceRetryWiring (M3)
- Reset failure_count to 0 when entering half-open from open (M6)
- Use cached _get_wait_strategy() in execute()/async_execute()
- Move circuit-open logging out of _on_failure lock scope to prevent
  holding the lock during potentially slow I/O (F1)
- Pass total_timeout=MAX_RETRY_TOTAL_TIMEOUT to wrap_service_method
  retry_service_operation call for consistency with execute() (F4)
- Capture failure_count into local variable inside lock scope before
  logging outside the lock, preventing stale reads from concurrent
  threads in CircuitBreaker.call() and async_call() (F1)
- Deep-copy module-level DEFAULT_DATABASE_RETRY and DEFAULT_CIRCUIT_BREAKER
  in ServiceRetryPolicyRegistry.get() for auto-generated unknown service
  policies, preventing shared mutable state corruption (F1)
- Unify CircuitBreaker to a single threading.Lock for sync and async (P1-1)
- Restore BaseException permit in half-open path to prevent permit leak (P1-6)
- Prevent CircuitBreakerOpen cascading into failure_count (S2)
- Protect all logger calls with contextlib.suppress (S3, S4)
- Replace time.time() with time.monotonic() for monotonic timing (S5)
- Add distinct log events for half-open and closed transitions (S11, S12)
- Track pre-existing services so second apply_settings_defaults only
  targets newly registered services (P1-2)
- Lazy circuit breaker creation via _get_or_create_cb() (P1-3)
- Reject async callables in sync execute() with TypeError (P1-5)
- Strengthen retry predicate to retry_if_exception_type(Exception) &
  retry_if_not_exception_type(CircuitBreakerOpen) (S1)
- Add lock on _get_wait_strategy cache access (P2-16)
- Truncate raw JSON to 80 chars in override warning (P2-17)
- Warn on non-dict JSON overrides (P2-29)
- Debug log for nesting guard bypass (S13)
- Deep-copy from get() and all_policies() in registry (P1-4)
- Thread-safe registry with threading.Lock (P2-15)
- Robust exception handling in apply_overrides get() (P2-18)
- Log ValidationError details on override failure (P2-19)
- Sanitize service_name via _safe_service_name() (P2-28)
- Warn on non-dict sub-key values in overrides (P2-30)
- Allowlist for is_read_only_plan_operation phases (P2-10)
- Cap retry_auto_debug sleep at 60s (P2-11)
- Use is-not-None instead of falsy checks for error values (P2-12)
- Extend secret regex with bearer, session_id, auth_token,
  refresh_token, client_secret patterns (P2-25)
- Pre-truncate error messages to 2000 chars before regex (P2-26)
- Add upper bounds on retry Settings fields (P2-7)
- Add cross-field validator max_delay >= base_delay (P2-8)
- Case-insensitive backoff strategy validation (P2-21)
- Add half_open_max_successes setting (S10)
- Remove phantom ContextFragment from services __all__ (ImportError fix)
- Export ServiceRetryWiring from application.services package
- Include sanitised error context in TypeError logging fallback
- Initialise RetryContext.attempt_count to 1 for bare context-manager usage
- Introduce CircuitBreakerState StrEnum replacing raw string literals
- Fix vacuous CircuitBreakerOpen propagation assertions in BDD steps
- Replace tautological logging test with structlog capture verification
- Assert circuit breaker existence instead of silently skipping on None
- Add Unicode control-char rejection validator to ServiceRetryPolicy.service_name
- Add name parameter with service= in all log calls
- Add extra="forbid" to all 3 Pydantic models
- Deep-copy _SERVICE_DEFAULTS construction
- Key normalisation (.strip()) in get() and apply_overrides()
- Add cooldown <= recovery_timeout validator
- Async guard on RetryContext.execute()
- Nesting guard on RetryContext.execute()/async_execute()
- stop_after_delay(300.0) on RetryContext
- retry_auto_debug async-only guard, dict result fix, sleep guard
- Retry-attempt logging in RetryContext
- Module-level docs for contextlib.suppress(TypeError) rationale
- Exhaustion log on retry failure
- Startup log in __init__; name=service_name to CircuitBreaker
- log_after_retry guarded to not fire on first-attempt success
- get_retry_decorator now includes logging callbacks
- Changed retry_backoff_strategy from str to RetryStrategy StrEnum

Closes #313
2026-03-11 17:42:13 +00:00

190 lines
7.0 KiB
Python

"""Step definitions added for review fixes S46, S48, S50.
S46: Parameterized sanitization Scenario Outline steps.
S50: Negative / edge-case scenarios for retry_policy_wiring.feature.
"""
from __future__ import annotations
import threading
from typing import Any
from behave import given, then, when
from pydantic import ValidationError
# ---------------------------------------------------------------------------
# S46: Parameterized sanitization Scenario Outline
# ---------------------------------------------------------------------------
_SANITIZE_INPUTS: dict[str, str] = {
"database_url": "Connection failed: postgres://admin:s3cret@db.host/mydb",
"api_key": "Auth failed: api_key=sk-12345abcdef token=xyz789",
"auth_header": "Request failed: Authorization: Bearer sk-live-abc123xyz",
"private_key": "Config error: private_key=AAAA-BBBB-CCCC-DDDD",
"connection_string": "DB error: connection_string=Server=db;Password=s3cret",
"access_key": "AWS error: access_key=AKIAIOSFODNN7EXAMPLE",
}
@when('I sanitize an error containing {sensitive_type} with secret "{secret}"')
def step_sanitize_parameterized(context: Any, sensitive_type: str, secret: str) -> None:
from cleveragents.core.retry_patterns import _sanitize_error_message
raw_msg = _SANITIZE_INPUTS[sensitive_type]
error = RuntimeError(raw_msg)
context.sanitized_outline = _sanitize_error_message(error)
context.expected_secret = secret
@then('the sanitized output should not contain "{secret}"')
def step_check_sanitized_no_secret(context: Any, secret: str) -> None:
assert secret not in context.sanitized_outline, (
f"Secret '{secret}' found in sanitized output: {context.sanitized_outline}"
)
@then('the sanitized output should contain "{redacted_marker}"')
def step_check_sanitized_has_marker(context: Any, redacted_marker: str) -> None:
assert redacted_marker in context.sanitized_outline, (
f"Redacted marker '{redacted_marker}' not found in: {context.sanitized_outline}"
)
# ---------------------------------------------------------------------------
# S50: Negative base_delay validation
# ---------------------------------------------------------------------------
@when("I attempt to create a RetryPolicyConfig with base_delay {value:g}")
def step_create_invalid_base_delay(context: Any, value: float) -> None:
from cleveragents.domain.models.core.retry_policy import RetryPolicyConfig
context.neg_validation_error = None
try:
RetryPolicyConfig(base_delay=value)
except ValidationError as exc:
context.neg_validation_error = exc
# ---------------------------------------------------------------------------
# S50: Disabled circuit breaker (enabled=false)
# ---------------------------------------------------------------------------
@given("I have a ServiceRetryWiring instance with disabled circuit breaker")
def step_create_wiring_disabled_cb(context: Any) -> None:
from cleveragents.application.services.service_retry_wiring import (
ServiceRetryWiring,
)
from cleveragents.config.settings import Settings
settings = Settings(
database_url="sqlite:///test_retry.db",
mock_providers=True,
env="test",
)
context.wiring = ServiceRetryWiring(settings)
# Disable the CB for plan_service by removing it
context.disabled_cb_service = "plan_service"
cb = context.wiring.get_circuit_breaker(context.disabled_cb_service)
if cb is not None:
# Simulate disabled CB by removing it from the map
context.wiring._circuit_breakers.pop(context.disabled_cb_service, None)
@when("I execute a function through the wiring for the disabled-CB service")
def step_execute_disabled_cb(context: Any) -> None:
try:
context.result = context.wiring.execute(
service_name=context.disabled_cb_service,
operation_name="disabled_cb_test",
func=lambda: "disabled_cb_ok",
idempotent=True,
)
context.exec_error = None
except Exception as exc:
context.exec_error = exc
context.result = None
@then("the function should execute successfully without circuit breaker protection")
def step_check_disabled_cb_success(context: Any) -> None:
assert context.exec_error is None, f"Unexpected error: {context.exec_error}"
assert context.result == "disabled_cb_ok"
# ---------------------------------------------------------------------------
# S50: Retry exhaustion with low CB threshold
# ---------------------------------------------------------------------------
@given("I have a ServiceRetryWiring instance with low CB threshold")
def step_create_wiring_low_cb_threshold(context: Any) -> None:
from cleveragents.application.services.service_retry_wiring import (
ServiceRetryWiring,
)
from cleveragents.config.settings import Settings
settings = Settings(
database_url="sqlite:///test_retry.db",
mock_providers=True,
env="test",
)
context.wiring = ServiceRetryWiring(settings)
# Lower the CB threshold to be <= max_attempts so exhaustion opens the CB
cb = context.wiring.get_circuit_breaker("plan_service")
if cb is not None:
cb.failure_threshold = 2 # With max_attempts=3, 3 failures will open it
# ---------------------------------------------------------------------------
# S50: Concurrent failures open the circuit breaker
# ---------------------------------------------------------------------------
@given("I have a standalone CircuitBreaker with threshold {threshold:d}")
def step_create_standalone_cb_with_threshold(context: Any, threshold: int) -> None:
from cleveragents.core.retry_patterns import CircuitBreaker
context.standalone_cb = CircuitBreaker(
failure_threshold=threshold,
expected_exception=Exception,
)
@when("{count:d} concurrent threads trigger failures on the circuit breaker")
def step_concurrent_cb_failures(context: Any, count: int) -> None:
barrier = threading.Barrier(count, timeout=5)
import contextlib
def trigger_failure() -> None:
barrier.wait()
with contextlib.suppress(Exception):
context.standalone_cb.call(_raise_runtime)
threads = [threading.Thread(target=trigger_failure) for _ in range(count)]
for t in threads:
t.start()
for t in threads:
t.join(timeout=5)
def _raise_runtime() -> str:
raise RuntimeError("concurrent failure")
# ---------------------------------------------------------------------------
# S50: Retry exhaustion opens circuit breaker
# ---------------------------------------------------------------------------
@then('the circuit breaker for "{name}" should be open after exhaustion')
def step_check_cb_open_after_exhaustion(context: Any, name: str) -> None:
cb = context.wiring.get_circuit_breaker(name)
assert cb is not None, f"No circuit breaker for {name}"
# After max_attempts failures, the CB should have opened
assert cb.state == "open", (
f"Expected CB state 'open' after exhaustion, got '{cb.state}'"
)