BUG-HUNT: [error-handling] A2aLocalFacade._handle_plan_apply() does not check plan phase before applying, causing InvalidPhaseTransitionError to propagate to callers #7389

New issue

Open

opened 2026-04-10 18:49:40 +00:00 by HAL9000 · 0 comments

HAL9000 commented

2026-04-10 18:49:40 +00:00

Owner

Bug Report: [error-handling] A2aLocalFacade._handle_plan_apply() missing idempotency guard unlike _handle_plan_execute()

Severity Assessment

Impact: The _handle_plan_apply() handler calls svc.apply_plan(plan_id) unconditionally, unlike _handle_plan_execute() which checks the current plan phase first for idempotency. If a plan is already in the Apply phase, calling apply_plan() again will raise InvalidPhaseTransitionError, which propagates as an unhandled error to the A2A caller. This breaks idempotency for apply operations.
Likelihood: Medium — occurs when apply is called twice (e.g., retry logic in orchestrators)
Priority: Medium

Location

File: src/cleveragents/a2a/facade.py
Function/Class: A2aLocalFacade._handle_plan_apply()
Lines: ~245-255

Description

Compare _handle_plan_execute() (which has an idempotency guard) with _handle_plan_apply() (which does not):

def _handle_plan_execute(self, params: dict[str, Any]) -> dict[str, Any]:
    svc = self._plan_lifecycle_service
    plan_id = params.get("plan_id", "")
    # ...
    # IDEMPOTENCY GUARD: checks current phase before executing
    plan = svc.get_plan(plan_id)
    if plan.phase.value in ("execute", "apply"):
        return {"plan_id": plan.identity.plan_id, "status": plan.phase.value}
    plan = svc.execute_plan(plan_id)
    return {"plan_id": plan.identity.plan_id, "status": plan.phase.value}

def _handle_plan_apply(self, params: dict[str, Any]) -> dict[str, Any]:
    svc = self._plan_lifecycle_service
    plan_id = params.get("plan_id", "")
    # ...
    # NO IDEMPOTENCY GUARD HERE! Will raise if plan is already applied
    plan = svc.apply_plan(plan_id)  # Throws if already applied!
    return {"plan_id": plan.identity.plan_id, "status": plan.phase.value}

The comment in _handle_plan_execute() explicitly references the idempotency pattern:

# If the plan has already reached (or passed) the execute phase,
# acknowledge idempotently instead of attempting a duplicate
# transition that would raise InvalidPhaseTransitionError.

The same logic is needed for _handle_plan_apply(). Without it:

User calls apply_plan → succeeds
User (or orchestrator) calls apply_plan again → InvalidPhaseTransitionError propagates, returning an error to the A2A caller
A2A caller may interpret this as a failure and retry, causing further errors

Evidence

def _handle_plan_execute(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": "queued"}
    if not plan_id:
        raise ValueError("plan_id is required")
    # CORRECT: idempotency guard
    plan = svc.get_plan(plan_id)
    if plan.phase.value in ("execute", "apply"):
        return {"plan_id": plan.identity.plan_id, "status": plan.phase.value}
    plan = svc.execute_plan(plan_id)
    return {"plan_id": plan.identity.plan_id, "status": plan.phase.value}

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"}
    if not plan_id:
        raise ValueError("plan_id is required")
    # BUG: missing idempotency guard!
    plan = svc.apply_plan(plan_id)  # Will throw InvalidPhaseTransitionError if already applied
    return {"plan_id": plan.identity.plan_id, "status": plan.phase.value}

Expected Behavior

_handle_plan_apply() should have the same idempotency guard as _handle_plan_execute():

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"}
    if not plan_id:
        raise ValueError("plan_id is required")
    # Idempotency guard: if already applied, acknowledge without re-applying
    plan = svc.get_plan(plan_id)
    if plan.phase.value == "apply":
        return {"plan_id": plan.identity.plan_id, "status": plan.phase.value}
    plan = svc.apply_plan(plan_id)
    return {"plan_id": plan.identity.plan_id, "status": plan.phase.value}

Actual Behavior

Calling apply on an already-applied plan raises InvalidPhaseTransitionError, which is caught by dispatch() and returned as an error response. Orchestrators expecting idempotent behavior will see an error on the second apply call.

TDD Note

After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: @tdd_issue, @tdd_issue_, and @tdd_expected_fail to prove the bug exists before fixing it.

Automated by CleverAgents Bot
Supervisor: Bug Detection Pool | Agent: bug-hunt-pool-supervisor

## Bug Report: [error-handling] A2aLocalFacade._handle_plan_apply() missing idempotency guard unlike _handle_plan_execute() ### Severity Assessment - **Impact**: The `_handle_plan_apply()` handler calls `svc.apply_plan(plan_id)` unconditionally, unlike `_handle_plan_execute()` which checks the current plan phase first for idempotency. If a plan is already in the Apply phase, calling `apply_plan()` again will raise `InvalidPhaseTransitionError`, which propagates as an unhandled error to the A2A caller. This breaks idempotency for apply operations. - **Likelihood**: Medium — occurs when apply is called twice (e.g., retry logic in orchestrators) - **Priority**: Medium ### Location - **File**: `src/cleveragents/a2a/facade.py` - **Function/Class**: `A2aLocalFacade._handle_plan_apply()` - **Lines**: ~245-255 ### Description Compare `_handle_plan_execute()` (which has an idempotency guard) with `_handle_plan_apply()` (which does not): ```python def _handle_plan_execute(self, params: dict[str, Any]) -> dict[str, Any]: svc = self._plan_lifecycle_service plan_id = params.get("plan_id", "") # ... # IDEMPOTENCY GUARD: checks current phase before executing plan = svc.get_plan(plan_id) if plan.phase.value in ("execute", "apply"): return {"plan_id": plan.identity.plan_id, "status": plan.phase.value} plan = svc.execute_plan(plan_id) return {"plan_id": plan.identity.plan_id, "status": plan.phase.value} def _handle_plan_apply(self, params: dict[str, Any]) -> dict[str, Any]: svc = self._plan_lifecycle_service plan_id = params.get("plan_id", "") # ... # NO IDEMPOTENCY GUARD HERE! Will raise if plan is already applied plan = svc.apply_plan(plan_id) # Throws if already applied! return {"plan_id": plan.identity.plan_id, "status": plan.phase.value} ``` The comment in `_handle_plan_execute()` explicitly references the idempotency pattern: ```python # If the plan has already reached (or passed) the execute phase, # acknowledge idempotently instead of attempting a duplicate # transition that would raise InvalidPhaseTransitionError. ``` The same logic is needed for `_handle_plan_apply()`. Without it: 1. User calls `apply_plan` → succeeds 2. User (or orchestrator) calls `apply_plan` again → `InvalidPhaseTransitionError` propagates, returning an error to the A2A caller 3. A2A caller may interpret this as a failure and retry, causing further errors ### Evidence ```python def _handle_plan_execute(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": "queued"} if not plan_id: raise ValueError("plan_id is required") # CORRECT: idempotency guard plan = svc.get_plan(plan_id) if plan.phase.value in ("execute", "apply"): return {"plan_id": plan.identity.plan_id, "status": plan.phase.value} plan = svc.execute_plan(plan_id) return {"plan_id": plan.identity.plan_id, "status": plan.phase.value} 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"} if not plan_id: raise ValueError("plan_id is required") # BUG: missing idempotency guard! plan = svc.apply_plan(plan_id) # Will throw InvalidPhaseTransitionError if already applied return {"plan_id": plan.identity.plan_id, "status": plan.phase.value} ``` ### Expected Behavior `_handle_plan_apply()` should have the same idempotency guard as `_handle_plan_execute()`: ```python 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"} if not plan_id: raise ValueError("plan_id is required") # Idempotency guard: if already applied, acknowledge without re-applying plan = svc.get_plan(plan_id) if plan.phase.value == "apply": return {"plan_id": plan.identity.plan_id, "status": plan.phase.value} plan = svc.apply_plan(plan_id) return {"plan_id": plan.identity.plan_id, "status": plan.phase.value} ``` ### Actual Behavior Calling `apply` on an already-applied plan raises `InvalidPhaseTransitionError`, which is caught by `dispatch()` and returned as an error response. Orchestrators expecting idempotent behavior will see an error on the second `apply` call. ### Category error-handling ### TDD Note After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: @tdd_issue, @tdd_issue_<this-issue-number>, and @tdd_expected_fail to prove the bug exists before fixing it. --- **Automated by CleverAgents Bot** Supervisor: Bug Detection Pool | Agent: bug-hunt-pool-supervisor

HAL9000 referenced this issue

2026-04-10 18:51:41 +00:00

[AUTO-BUG-SUP] Bug Hunt Status (Cycle 4) #7384

HAL9000 referenced this issue from a commit

2026-04-10 19:05:21 +00:00

docs(spec): clarify fail_fast cancel semantics and A2A facade idempotency

~~HAL9000 referenced this issue 2026-04-10 19:05:53 +00:00~~

docs(spec): clarify fail_fast cancel semantics and A2A facade idempotency #7410

HAL9000 referenced this issue

2026-04-10 19:06:48 +00:00

[AUTO-ARCH] Architecture Status (Cycle 1) #7343

HAL9000 referenced this issue