UAT: ErrorRecoveryService.record_error() missing fail-fast argument validation for plan_id, phase, and message #3737

New issue

Open

opened 2026-04-05 22:22:50 +00:00 by freemo · 0 comments

freemo commented

2026-04-05 22:22:50 +00:00

Owner

Metadata

Branch: fix/error-recovery-service-argument-validation
Commit Message: fix(services): add fail-fast argument validation to ErrorRecoveryService.record_error()
Milestone: (none — backlog)
Parent Epic: #394

Background

The specification (CONTRIBUTING.md) requires: "All public and protected methods must validate their arguments as the first step of execution (fail-fast). This includes checks for value ranges, nulls, expected types, and empty strings/collections."

ErrorRecoveryService.record_error() in src/cleveragents/application/services/error_recovery_service.py does not validate its required arguments (plan_id, phase, message) before use. Passing empty strings will silently create malformed error records with blank identifiers, corrupting the error history.

Affected File

src/cleveragents/application/services/error_recovery_service.py

Code Location

ErrorRecoveryService.record_error() (lines ~60-115):

def record_error(
    self,
    plan_id: str,
    phase: str,
    message: str,
    *,
    exception: BaseException | None = None,
    exception_type: str | None = None,
    actor: str | None = None,
    tool_call: str | None = None,
    category: ErrorCategory | None = None,
) -> ErrorRecord:
    """Record an error event for a plan."""
    # Classify
    exc_type_name = exception_type
    ...

No validation of plan_id, phase, or message before use.

Steps to Reproduce

Instantiate ErrorRecoveryService(lifecycle_service=mock_lifecycle)
Call service.record_error("", "execute", "some error") — no exception raised
Call service.record_error("valid-plan-id", "", "some error") — no exception raised
Call service.record_error("valid-plan-id", "execute", "") — no exception raised

Expected Behaviour

record_error() should raise ValidationError (from cleveragents.core.exceptions) immediately when:

plan_id is empty or whitespace-only
phase is empty or whitespace-only
message is empty or whitespace-only

Actual Behaviour

No validation is performed. Empty/blank values are silently accepted, creating malformed ErrorRecord objects with blank identifiers that corrupt the error history and may cause downstream failures in format_error_output() and should_retry().

Subtasks

Add fail-fast validation at the start of record_error() for plan_id, phase, and message
Add Behave scenarios verifying ValidationError is raised for each empty argument
Verify nox -e unit_tests passes

Definition of Done

record_error() raises ValidationError for empty/whitespace plan_id, phase, or message
Behave tests cover the validation cases
nox -e unit_tests passes
PR merged

Backlog note: This issue was discovered during autonomous operation
on milestone v3.3.0. It does not block milestone completion and has been
placed in the backlog for human review and future milestone assignment.

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/error-recovery-service-argument-validation` - **Commit Message**: `fix(services): add fail-fast argument validation to ErrorRecoveryService.record_error()` - **Milestone**: *(none — backlog)* - **Parent Epic**: #394 ## Background The specification (CONTRIBUTING.md) requires: *"All public and protected methods must validate their arguments as the first step of execution (fail-fast). This includes checks for value ranges, nulls, expected types, and empty strings/collections."* `ErrorRecoveryService.record_error()` in `src/cleveragents/application/services/error_recovery_service.py` does **not** validate its required arguments (`plan_id`, `phase`, `message`) before use. Passing empty strings will silently create malformed error records with blank identifiers, corrupting the error history. ## Affected File - `src/cleveragents/application/services/error_recovery_service.py` ## Code Location `ErrorRecoveryService.record_error()` (lines ~60-115): ```python def record_error( self, plan_id: str, phase: str, message: str, *, exception: BaseException | None = None, exception_type: str | None = None, actor: str | None = None, tool_call: str | None = None, category: ErrorCategory | None = None, ) -> ErrorRecord: """Record an error event for a plan.""" # Classify exc_type_name = exception_type ... ``` No validation of `plan_id`, `phase`, or `message` before use. ## Steps to Reproduce 1. Instantiate `ErrorRecoveryService(lifecycle_service=mock_lifecycle)` 2. Call `service.record_error("", "execute", "some error")` — no exception raised 3. Call `service.record_error("valid-plan-id", "", "some error")` — no exception raised 4. Call `service.record_error("valid-plan-id", "execute", "")` — no exception raised ## Expected Behaviour `record_error()` should raise `ValidationError` (from `cleveragents.core.exceptions`) immediately when: - `plan_id` is empty or whitespace-only - `phase` is empty or whitespace-only - `message` is empty or whitespace-only ## Actual Behaviour No validation is performed. Empty/blank values are silently accepted, creating malformed `ErrorRecord` objects with blank identifiers that corrupt the error history and may cause downstream failures in `format_error_output()` and `should_retry()`. ## Subtasks - [ ] Add fail-fast validation at the start of `record_error()` for `plan_id`, `phase`, and `message` - [ ] Add Behave scenarios verifying `ValidationError` is raised for each empty argument - [ ] Verify `nox -e unit_tests` passes ## Definition of Done - `record_error()` raises `ValidationError` for empty/whitespace `plan_id`, `phase`, or `message` - Behave tests cover the validation cases - `nox -e unit_tests` passes - PR merged > **Backlog note:** This issue was discovered during autonomous operation > on milestone v3.3.0. It does not block milestone completion and has been > placed in the backlog for human review and future milestone assignment. --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator

freemo added a new dependency

2026-04-05 22:23:04 +00:00

#394 Epic: Decision Framework

freemo referenced this issue

2026-04-05 22:29:07 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 (Resumed) #3654

freemo referenced this issue

2026-04-05 22:29:26 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 (Resumed) #3654

freemo referenced this issue

2026-04-05 22:33:37 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 (Resumed) #3654