cleveragents/cleveragents-core

Fork 3

BUG-HUNT: [boundary-condition] _handle_plan_cancel/_handle_plan_tree/_handle_plan_artifacts return fake success for empty plan_id even when real service is available #7738

New issue

Open

opened 2026-04-12 03:22:13 +00:00 by HAL9000 · 3 comments

HAL9000 commented

2026-04-12 03:22:13 +00:00

Owner

Bug Report: [Boundary Condition] — Empty plan_id Returns Fake Success in cancel/tree/artifacts Handlers

Severity Assessment

Impact: Callers that accidentally send an empty plan_id to _cleveragents/plan/cancel, _cleveragents/plan/tree, or _cleveragents/plan/artifacts receive a fake success response instead of a validation error. No cancellation or data retrieval actually occurs, but the caller believes it succeeded.
Likelihood: Medium — any client bug that omits plan_id silently proceeds with incorrect behavior
Priority: High

Location

File: src/cleveragents/a2a/facade.py
Function/Class: _handle_plan_cancel(), _handle_plan_tree(), _handle_plan_artifacts()
Lines: ~290–340

Description

Three handlers use a combined if svc is None or not plan_id: guard that conflates two distinct conditions — service unavailability (stub mode) and missing input validation. When the real service is available (svc is not None) but plan_id is empty, not plan_id is True and the combined condition short-circuits to return a fake stub response:

_handle_plan_cancel() returns {"status": "cancelled", "stub": True} — caller thinks plan was cancelled
_handle_plan_tree() returns {"tree": [], "stub": True} — caller thinks plan has no decisions
_handle_plan_artifacts() returns {"artifacts": [], "stub": True} — caller thinks plan has no artifacts

This is inconsistent with _handle_plan_execute() and _handle_plan_apply(), which correctly separate the two conditions.

Evidence

# BUGGY — facade.py _handle_plan_cancel():
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:              # <-- WRONG: conflated guard
        return {"plan_id": plan_id, "status": "cancelled", "stub": True}
    svc.cancel_plan(plan_id)
    return {"plan_id": plan_id, "status": "cancelled"}

# Same bug in _handle_plan_tree() and _handle_plan_artifacts().

# CORRECT pattern — _handle_plan_apply():
def _handle_plan_apply(self, params: dict[str, Any]) -> dict[str, Any]:
    svc = self._plan_lifecycle_service
    plan_id = params.get("plan_id", "")
    if svc is None:
        return {"plan_id": plan_id, "status": "applied"}  # stub only when no service
    if not plan_id:                              # <-- CORRECT: separate validation
        raise ValueError("plan_id is required")
    plan = svc.apply_plan(plan_id)
    return {"plan_id": plan.identity.plan_id, "status": plan.phase.value}

Expected Behavior

When plan_id is empty and the real service is available, handlers should raise ValueError("plan_id is required") — just as _handle_plan_execute() and _handle_plan_apply() do. The stub fallback should only apply when the service is absent.

Actual Behavior

With a real service registered and plan_id = "", all three handlers return a fake {"stub": True} success response. No operation is performed on the service.

Suggested Fix

Apply the same two-step guard used by _handle_plan_execute() and _handle_plan_apply():

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:
        return {"plan_id": plan_id, "status": "cancelled", "stub": True}
    if not plan_id:
        raise ValueError("plan_id is required")
    svc.cancel_plan(plan_id)
    return {"plan_id": plan_id, "status": "cancelled"}

# Apply same fix to _handle_plan_tree() and _handle_plan_artifacts()

TDD Note

After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD.

Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: bug-hunter

## Bug Report: [Boundary Condition] — Empty plan_id Returns Fake Success in cancel/tree/artifacts Handlers ### Severity Assessment - **Impact**: Callers that accidentally send an empty `plan_id` to `_cleveragents/plan/cancel`, `_cleveragents/plan/tree`, or `_cleveragents/plan/artifacts` receive a fake success response instead of a validation error. No cancellation or data retrieval actually occurs, but the caller believes it succeeded. - **Likelihood**: Medium — any client bug that omits `plan_id` silently proceeds with incorrect behavior - **Priority**: High ### Location - **File**: `src/cleveragents/a2a/facade.py` - **Function/Class**: `_handle_plan_cancel()`, `_handle_plan_tree()`, `_handle_plan_artifacts()` - **Lines**: ~290–340 ### Description Three handlers use a combined `if svc is None or not plan_id:` guard that conflates two distinct conditions — service unavailability (stub mode) and missing input validation. When the real service is available (`svc is not None`) but `plan_id` is empty, `not plan_id` is `True` and the combined condition short-circuits to return a fake stub response: - `_handle_plan_cancel()` returns `{"status": "cancelled", "stub": True}` — caller thinks plan was cancelled - `_handle_plan_tree()` returns `{"tree": [], "stub": True}` — caller thinks plan has no decisions - `_handle_plan_artifacts()` returns `{"artifacts": [], "stub": True}` — caller thinks plan has no artifacts This is inconsistent with `_handle_plan_execute()` and `_handle_plan_apply()`, which **correctly** separate the two conditions. ### Evidence ```python # BUGGY — facade.py _handle_plan_cancel(): 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: # <-- WRONG: conflated guard return {"plan_id": plan_id, "status": "cancelled", "stub": True} svc.cancel_plan(plan_id) return {"plan_id": plan_id, "status": "cancelled"} # Same bug in _handle_plan_tree() and _handle_plan_artifacts(). # CORRECT pattern — _handle_plan_apply(): def _handle_plan_apply(self, params: dict[str, Any]) -> dict[str, Any]: svc = self._plan_lifecycle_service plan_id = params.get("plan_id", "") if svc is None: return {"plan_id": plan_id, "status": "applied"} # stub only when no service if not plan_id: # <-- CORRECT: separate validation raise ValueError("plan_id is required") plan = svc.apply_plan(plan_id) return {"plan_id": plan.identity.plan_id, "status": plan.phase.value} ``` ### Expected Behavior When `plan_id` is empty and the real service is available, handlers should raise `ValueError("plan_id is required")` — just as `_handle_plan_execute()` and `_handle_plan_apply()` do. The stub fallback should only apply when the service is absent. ### Actual Behavior With a real service registered and `plan_id = ""`, all three handlers return a fake `{"stub": True}` success response. No operation is performed on the service. ### Suggested Fix Apply the same two-step guard used by `_handle_plan_execute()` and `_handle_plan_apply()`: ```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: return {"plan_id": plan_id, "status": "cancelled", "stub": True} if not plan_id: raise ValueError("plan_id is required") svc.cancel_plan(plan_id) return {"plan_id": plan_id, "status": "cancelled"} # Apply same fix to _handle_plan_tree() and _handle_plan_artifacts() ``` ### Category boundary-condition ### TDD Note After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: bug-hunter

HAL9000 added this to the v3.2.0 milestone

2026-04-12 03:44:25 +00:00

HAL9000 referenced this issue

2026-04-12 03:47:56 +00:00

[AUTO-PROJ-OWN] Project Owner Report (Cycle 1) #7724

HAL9000 added the

labels

2026-04-12 07:15:56 +00:00

freemo added the

Type

Bug

label

2026-04-12 18:33:50 +00:00

HAL9000 added

and removed

labels

2026-04-14 05:58:41 +00:00

HAL9000 commented

2026-04-14 05:58:41 +00:00

Author

Owner

✅ Verified — Bug: handlers return fake success for empty plan_id. MoSCoW: Must-have. Priority: High — misleading behavior.

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

✅ **Verified** — Bug: handlers return fake success for empty plan_id. MoSCoW: Must-have. Priority: High — misleading behavior. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 commented

2026-04-14 06:11:57 +00:00

Author

Owner

✅ Verified — Bug: handlers return fake success for empty plan_id. MoSCoW: Must-have. Priority: High — misleading behavior.

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