TDD: A2aLocalFacade plan handlers inconsistently validate empty plan_id — cancel/tree/artifacts silently return stub instead of raising ValueError #10389

New issue

Open

opened 2026-04-18 09:25:13 +00:00 by HAL9000 · 0 comments

HAL9000 commented

2026-04-18 09:25:13 +00:00

Owner

Metadata

Field	Value
Branch	`test/a2a-facade-plan-id-validation-tdd`
Commit Message	`test(a2a/facade): add @tdd_expected_fail scenarios for inconsistent plan_id validation in cancel/tree/artifacts handlers`
Milestone	v3.5.0
Parent Epic	—

Background and Context

A2aLocalFacade in src/cleveragents/a2a/facade.py has inconsistent plan_id validation across its plan operation handlers. Some handlers raise ValueError("plan_id is required") when plan_id is empty, while others silently return stub responses — even when a real service is wired.

Handlers that raise ValueError for empty plan_id (correct behavior):

_handle_plan_execute: if not plan_id: raise ValueError("plan_id is required")
_handle_plan_status: if not plan_id: raise ValueError("plan_id is required")
_handle_plan_diff: if not plan_id: raise ValueError("plan_id is required")
_handle_plan_apply: if not plan_id: raise ValueError("plan_id is required")

Handlers that silently return stub for empty plan_id (incorrect behavior):

def _handle_plan_cancel(self, params: dict[str, Any]) -> dict[str, Any]:
    plan_id = params.get("plan_id", "")
    svc = self._plan_lifecycle_service
    if svc is None or not plan_id:  # ← returns stub even when svc is wired
        return {"plan_id": plan_id, "status": "cancelled", "stub": True}
    svc.cancel_plan(plan_id)
    return {"plan_id": plan_id, "status": "cancelled"}

def _handle_plan_tree(self, params: dict[str, Any]) -> dict[str, Any]:
    plan_id = params.get("plan_id", "")
    svc = self._plan_lifecycle_service
    if svc is None or not plan_id:  # ← same issue
        return {"plan_id": plan_id, "tree": [], "stub": True}
    ...

def _handle_plan_artifacts(self, params: dict[str, Any]) -> dict[str, Any]:
    plan_id = params.get("plan_id", "")
    svc = self._plan_lifecycle_service
    if svc is None or not plan_id:  # ← same issue
        return {"plan_id": plan_id, "artifacts": [], "stub": True}
    ...

When a service IS wired and plan_id is empty, these handlers return {"status": "cancelled", "stub": True} — falsely claiming the operation succeeded. This is a silent incorrect behavior that masks programming errors in callers.

Expected Behavior

A Behave scenario tagged @tdd_issue @tdd_issue_N @tdd_expected_fail should:

Create an A2aLocalFacade with a mock plan_lifecycle_service wired
Call dispatch() with _cleveragents/plan/cancel (or tree or artifacts) and empty plan_id
Assert that the returned A2aResponse has error set (not result)
Assert that the error indicates plan_id is required

This test must fail on the current codebase (because the handlers return stub success responses), confirming the bug exists.

Acceptance Criteria

A Behave scenario tagged @tdd_issue, @tdd_issue_N, and @tdd_expected_fail exists for _cleveragents/plan/cancel with empty plan_id
A Behave scenario exists for _cleveragents/plan/tree with empty plan_id
A Behave scenario exists for _cleveragents/plan/artifacts with empty plan_id
All scenarios assert that an error response is returned (not a stub success)
All scenarios fail on the current codebase (confirming the bug)
All scenarios pass after the fix is applied

Subtasks

Create feature file with scenarios for each affected handler
Tag all scenarios with @tdd_issue @tdd_issue_N @tdd_expected_fail
Implement step definitions
Verify scenarios fail on the current codebase
Run nox -s unit_tests to confirm no regressions

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
The failing Behave scenarios are committed on the branch test/a2a-facade-plan-id-validation-tdd.
All scenarios are tagged correctly.
Scenarios fail before the fix and pass after the fix is applied.

Automated by CleverAgents Bot
Agent: new-issue-creator

## Metadata | Field | Value | |---|---| | **Branch** | `test/a2a-facade-plan-id-validation-tdd` | | **Commit Message** | `test(a2a/facade): add @tdd_expected_fail scenarios for inconsistent plan_id validation in cancel/tree/artifacts handlers` | | **Milestone** | v3.5.0 | | **Parent Epic** | — | ## Background and Context `A2aLocalFacade` in `src/cleveragents/a2a/facade.py` has inconsistent `plan_id` validation across its plan operation handlers. Some handlers raise `ValueError("plan_id is required")` when `plan_id` is empty, while others silently return stub responses — even when a real service is wired. **Handlers that raise ValueError for empty plan_id** (correct behavior): - `_handle_plan_execute`: `if not plan_id: raise ValueError("plan_id is required")` - `_handle_plan_status`: `if not plan_id: raise ValueError("plan_id is required")` - `_handle_plan_diff`: `if not plan_id: raise ValueError("plan_id is required")` - `_handle_plan_apply`: `if not plan_id: raise ValueError("plan_id is required")` **Handlers that silently return stub for empty plan_id** (incorrect behavior): ```python def _handle_plan_cancel(self, params: dict[str, Any]) -> dict[str, Any]: plan_id = params.get("plan_id", "") svc = self._plan_lifecycle_service if svc is None or not plan_id: # ← returns stub even when svc is wired return {"plan_id": plan_id, "status": "cancelled", "stub": True} svc.cancel_plan(plan_id) return {"plan_id": plan_id, "status": "cancelled"} def _handle_plan_tree(self, params: dict[str, Any]) -> dict[str, Any]: plan_id = params.get("plan_id", "") svc = self._plan_lifecycle_service if svc is None or not plan_id: # ← same issue return {"plan_id": plan_id, "tree": [], "stub": True} ... def _handle_plan_artifacts(self, params: dict[str, Any]) -> dict[str, Any]: plan_id = params.get("plan_id", "") svc = self._plan_lifecycle_service if svc is None or not plan_id: # ← same issue return {"plan_id": plan_id, "artifacts": [], "stub": True} ... ``` When a service IS wired and `plan_id` is empty, these handlers return `{"status": "cancelled", "stub": True}` — falsely claiming the operation succeeded. This is a silent incorrect behavior that masks programming errors in callers. ## Expected Behavior A Behave scenario tagged `@tdd_issue @tdd_issue_N @tdd_expected_fail` should: 1. Create an `A2aLocalFacade` with a mock `plan_lifecycle_service` wired 2. Call `dispatch()` with `_cleveragents/plan/cancel` (or `tree` or `artifacts`) and empty `plan_id` 3. Assert that the returned `A2aResponse` has `error` set (not `result`) 4. Assert that the error indicates `plan_id` is required This test **must fail** on the current codebase (because the handlers return stub success responses), confirming the bug exists. ## Acceptance Criteria - [ ] A Behave scenario tagged `@tdd_issue`, `@tdd_issue_N`, and `@tdd_expected_fail` exists for `_cleveragents/plan/cancel` with empty plan_id - [ ] A Behave scenario exists for `_cleveragents/plan/tree` with empty plan_id - [ ] A Behave scenario exists for `_cleveragents/plan/artifacts` with empty plan_id - [ ] All scenarios assert that an error response is returned (not a stub success) - [ ] All scenarios **fail** on the current codebase (confirming the bug) - [ ] All scenarios **pass** after the fix is applied ## Subtasks - [ ] Create feature file with scenarios for each affected handler - [ ] Tag all scenarios with `@tdd_issue @tdd_issue_N @tdd_expected_fail` - [ ] Implement step definitions - [ ] Verify scenarios fail on the current codebase - [ ] Run `nox -s unit_tests` to confirm no regressions ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - The failing Behave scenarios are committed on the branch `test/a2a-facade-plan-id-validation-tdd`. - All scenarios are tagged correctly. - Scenarios fail before the fix and pass after the fix is applied. --- **Automated by CleverAgents Bot** Agent: new-issue-creator

HAL9000 added the

labels

2026-04-18 09:26:44 +00:00

HAL9000 referenced this issue

2026-04-18 09:32:19 +00:00

[AUTO-WATCHDOG] Status: RUNNING (Cycle 9) #10404