cleveragents/cleveragents-core

Fork 3

UAT: A2A facade maps Python ValueError to INTERNAL_ERROR instead of VALIDATION_ERROR — incorrect error classification #1442

New issue

Open

opened 2026-04-02 17:58:24 +00:00 by freemo · 1 comment

freemo commented

2026-04-02 17:58:24 +00:00

Owner

Metadata

Branch: bugfix/m5-a2a-valueerror-validation-error-mapping
Commit Message: fix(a2a): map ValueError to VALIDATION_ERROR in map_domain_error
Milestone: v3.4.0
Parent Epic: #933

Bug Report

Feature Area: A2A Protocol Integration
Severity: Low
Found by: UAT tester uat-worker-a2a-protocol-2

What Was Tested

Verified that A2A error responses use the correct error codes when facade handlers raise validation errors for missing required parameters.

Expected Behavior (from spec)

When a required parameter like plan_id is missing from an A2A request, the error response should use the VALIDATION_ERROR code to indicate a client-side input problem.

Actual Behavior

The facade handlers (e.g., _handle_plan_execute, _handle_plan_status, _handle_plan_apply) raise Python's built-in ValueError when required parameters are missing:

# In src/cleveragents/a2a/facade.py
def _handle_plan_execute(self, params: dict[str, Any]) -> dict[str, Any]:
    plan_id = params.get("plan_id", "")
    if not plan_id:
        raise ValueError("plan_id is required")  # Python ValueError

However, map_domain_error() in src/cleveragents/a2a/errors.py only maps the domain ValidationError class to VALIDATION_ERROR. Python's built-in ValueError falls through to the generic INTERNAL_ERROR mapping:

def map_domain_error(exc: Exception) -> tuple[str, str]:
    if isinstance(exc, ResourceNotFoundError):
        return NOT_FOUND, str(exc)
    if isinstance(exc, ValidationError):  # domain ValidationError only
        return VALIDATION_ERROR, str(exc)
    # ...
    return INTERNAL_ERROR, str(exc)  # ValueError falls here

Steps to Reproduce

from cleveragents.a2a.facade import A2aLocalFacade
from cleveragents.a2a.models import A2aRequest

class MockPlanService:
    def execute_plan(self, plan_id):
        if not plan_id:
            raise ValueError('plan_id is required')
        ...

facade = A2aLocalFacade(services={'plan_lifecycle_service': MockPlanService()})
req = A2aRequest(operation='_cleveragents/plan/execute', params={})  # No plan_id
resp = facade.dispatch(req)
print(resp.error.code)  # "INTERNAL_ERROR" — should be "VALIDATION_ERROR"

Code Location

src/cleveragents/a2a/errors.py — map_domain_error() function
src/cleveragents/a2a/facade.py — _handle_plan_execute, _handle_plan_status, _handle_plan_apply, _handle_session_close

Fix Options

Add ValueError mapping to map_domain_error():

if isinstance(exc, (ValidationError, ValueError)):
    return VALIDATION_ERROR, str(exc)

Change facade handlers to raise domain ValidationError instead of Python ValueError

Subtasks

Write a failing Behave BDD scenario (@tdd_issue, @tdd_expected_fail) that reproduces the incorrect INTERNAL_ERROR code when ValueError is raised for a missing required parameter
Choose and implement a fix: either extend map_domain_error() to handle ValueError, or update facade handlers to raise domain ValidationError
Update _handle_plan_execute, _handle_plan_status, _handle_plan_apply, and _handle_session_close in src/cleveragents/a2a/facade.py if Option 2 is chosen
Remove @tdd_expected_fail tag once the fix is in place and the scenario passes
Tests (Behave): Add scenarios for each affected handler (plan/execute, plan/status, plan/apply, session/close) with missing required params
Run nox -e typecheck — confirm 0 errors
Run nox -e unit_tests — confirm all Behave scenarios pass
Run nox -e coverage_report — confirm coverage ≥ 97%
Run nox (all default sessions), fix any errors

Definition of Done

All subtasks are completed and checked off
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done
map_domain_error() returns VALIDATION_ERROR (not INTERNAL_ERROR) when a missing required parameter causes a ValueError in any A2A facade handler
BDD scenarios cover all four affected handlers with missing required params
All nox stages pass
Coverage >= 97%

## Metadata - **Branch**: `bugfix/m5-a2a-valueerror-validation-error-mapping` - **Commit Message**: `fix(a2a): map ValueError to VALIDATION_ERROR in map_domain_error` - **Milestone**: v3.4.0 - **Parent Epic**: #933 ## Bug Report **Feature Area:** A2A Protocol Integration **Severity:** Low **Found by:** UAT tester uat-worker-a2a-protocol-2 ### What Was Tested Verified that A2A error responses use the correct error codes when facade handlers raise validation errors for missing required parameters. ### Expected Behavior (from spec) When a required parameter like `plan_id` is missing from an A2A request, the error response should use the `VALIDATION_ERROR` code to indicate a client-side input problem. ### Actual Behavior The facade handlers (e.g., `_handle_plan_execute`, `_handle_plan_status`, `_handle_plan_apply`) raise Python's built-in `ValueError` when required parameters are missing: ```python # In src/cleveragents/a2a/facade.py def _handle_plan_execute(self, params: dict[str, Any]) -> dict[str, Any]: plan_id = params.get("plan_id", "") if not plan_id: raise ValueError("plan_id is required") # Python ValueError ``` However, `map_domain_error()` in `src/cleveragents/a2a/errors.py` only maps the **domain** `ValidationError` class to `VALIDATION_ERROR`. Python's built-in `ValueError` falls through to the generic `INTERNAL_ERROR` mapping: ```python def map_domain_error(exc: Exception) -> tuple[str, str]: if isinstance(exc, ResourceNotFoundError): return NOT_FOUND, str(exc) if isinstance(exc, ValidationError): # domain ValidationError only return VALIDATION_ERROR, str(exc) # ... return INTERNAL_ERROR, str(exc) # ValueError falls here ``` ### Steps to Reproduce ```python from cleveragents.a2a.facade import A2aLocalFacade from cleveragents.a2a.models import A2aRequest class MockPlanService: def execute_plan(self, plan_id): if not plan_id: raise ValueError('plan_id is required') ... facade = A2aLocalFacade(services={'plan_lifecycle_service': MockPlanService()}) req = A2aRequest(operation='_cleveragents/plan/execute', params={}) # No plan_id resp = facade.dispatch(req) print(resp.error.code) # "INTERNAL_ERROR" — should be "VALIDATION_ERROR" ``` ### Code Location - `src/cleveragents/a2a/errors.py` — `map_domain_error()` function - `src/cleveragents/a2a/facade.py` — `_handle_plan_execute`, `_handle_plan_status`, `_handle_plan_apply`, `_handle_session_close` ### Fix Options 1. Add `ValueError` mapping to `map_domain_error()`: ```python if isinstance(exc, (ValidationError, ValueError)): return VALIDATION_ERROR, str(exc) ``` 2. Change facade handlers to raise domain `ValidationError` instead of Python `ValueError` ## Subtasks - [ ] Write a failing Behave BDD scenario (`@tdd_issue`, `@tdd_expected_fail`) that reproduces the incorrect `INTERNAL_ERROR` code when `ValueError` is raised for a missing required parameter - [ ] Choose and implement a fix: either extend `map_domain_error()` to handle `ValueError`, or update facade handlers to raise domain `ValidationError` - [ ] Update `_handle_plan_execute`, `_handle_plan_status`, `_handle_plan_apply`, and `_handle_session_close` in `src/cleveragents/a2a/facade.py` if Option 2 is chosen - [ ] Remove `@tdd_expected_fail` tag once the fix is in place and the scenario passes - [ ] Tests (Behave): Add scenarios for each affected handler (`plan/execute`, `plan/status`, `plan/apply`, `session/close`) with missing required params - [ ] Run `nox -e typecheck` — confirm 0 errors - [ ] Run `nox -e unit_tests` — confirm all Behave scenarios pass - [ ] Run `nox -e coverage_report` — confirm coverage ≥ 97% - [ ] Run `nox` (all default sessions), fix any errors ## Definition of Done - [ ] All subtasks are completed and checked off - [ ] A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation - [ ] The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly - [ ] The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done - [ ] `map_domain_error()` returns `VALIDATION_ERROR` (not `INTERNAL_ERROR`) when a missing required parameter causes a `ValueError` in any A2A facade handler - [ ] BDD scenarios cover all four affected handlers with missing required params - [ ] All nox stages pass - [ ] Coverage >= 97%

freemo added the

labels

2026-04-02 17:59:14 +00:00

freemo added this to the v3.4.0 milestone

2026-04-02 17:59:54 +00:00

freemo referenced this issue

2026-04-02 18:22:31 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 11 — v3.7.0 TUI #1453

freemo referenced this issue

2026-04-02 18:27:35 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 21 — v3.7.0 TUI #1454

freemo referenced this issue

2026-04-02 18:32:39 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 31 — v3.7.0 TUI #1455

freemo referenced this issue

2026-04-02 18:37:41 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 41 — v3.7.0 TUI #1456

freemo referenced this issue

2026-04-02 18:42:43 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 51 — v3.7.0 TUI #1457

freemo self-assigned this

2026-04-02 18:45:09 +00:00

freemo referenced this issue

2026-04-02 18:47:45 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 61 — v3.7.0 TUI #1459

freemo referenced this issue

2026-04-02 18:52:49 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 71 — v3.7.0 TUI #1460

freemo referenced this issue

2026-04-02 18:57:50 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 81 — v3.7.0 TUI #1462

freemo referenced this issue

2026-04-02 19:02:53 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 91 — v3.7.0 TUI #1464

freemo referenced this issue

2026-04-02 19:07:59 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 101 — v3.7.0 TUI #1467

freemo referenced this issue

2026-04-02 19:13:05 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 111 — v3.7.0 TUI #1470

freemo referenced this issue

2026-04-02 19:18:07 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 121 — v3.7.0 TUI #1474

freemo referenced this issue

2026-04-02 19:23:10 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 131 — v3.7.0 TUI #1477

freemo referenced this issue

2026-04-02 19:28:13 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 141 — v3.7.0 TUI #1478

freemo commented

2026-04-02 19:30:49 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: Low (already assigned) — Error classification issue. The system still returns an error, just with the wrong code.
Milestone: v3.4.0 (already assigned)
MoSCoW: Could Have (already assigned) — While incorrect error codes are a spec deviation, the system still correctly rejects invalid input. The user sees an error either way. This is an internal API quality improvement, not a user-facing functional issue.
Parent Epic: #933

Valid bug. The fix is straightforward — either add ValueError to the mapping in map_domain_error() or change facade handlers to raise domain ValidationError.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **Priority**: Low (already assigned) — Error classification issue. The system still returns an error, just with the wrong code. - **Milestone**: v3.4.0 (already assigned) - **MoSCoW**: Could Have (already assigned) — While incorrect error codes are a spec deviation, the system still correctly rejects invalid input. The user sees an error either way. This is an internal API quality improvement, not a user-facing functional issue. - **Parent Epic**: #933 Valid bug. The fix is straightforward — either add `ValueError` to the mapping in `map_domain_error()` or change facade handlers to raise domain `ValidationError`. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner

freemo referenced this issue

2026-04-02 19:33:17 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 151 — v3.7.0 TUI #1497

freemo referenced this issue

2026-04-02 19:38:20 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 161 — v3.7.0 TUI #1503

freemo referenced this issue

2026-04-02 19:43:28 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 171 — v3.7.0 TUI #1505

freemo referenced this issue

2026-04-02 19:48:30 +00:00

[UAT-SUPERVISOR] Progress Report — Cycle 181 — v3.7.0 TUI #1507