cleveragents/cleveragents-core

Fork 3

BUG-HUNT: [spec-alignment] A2aLocalFacade._handle_plan_execute() does not check plan phase before executing — wrong phase transition guard #7529

New issue

Open

opened 2026-04-10 21:36:56 +00:00 by HAL9000 · 3 comments

HAL9000 commented

2026-04-10 21:36:56 +00:00

Owner

Bug Report: [spec-alignment] — A2aLocalFacade._handle_plan_execute() Phase Guard Uses Wrong Condition

Severity Assessment

Impact: The phase check in _handle_plan_execute() skips execution if the plan is already in execute or apply phase. However it should also skip (or raise) if the plan is in a terminal phase (cancelled, errored, completed). As written, a cancelled or errored plan passed to _cleveragents/plan/execute will attempt svc.execute_plan() and raise an unhandled domain error that bubbles up as an unspecified A2A error code.
Likelihood: Medium — affects idempotency in automated retry scenarios.
Priority: Medium

Location

File: src/cleveragents/a2a/facade.py
Function/Class: A2aLocalFacade._handle_plan_execute
Lines: 351–363

Description

_handle_plan_execute() guards against duplicate transitions:

def _handle_plan_execute(self, params: dict[str, Any]) -> dict[str, Any]:
    svc = self._plan_lifecycle_service
    plan_id = params.get("plan_id", "")
    ...
    plan = svc.get_plan(plan_id)
    if plan.phase.value in ("execute", "apply"):   # INCOMPLETE GUARD
        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}

The guard only checks for execute and apply phases. The spec defines additional terminal phases: cancelled, completed (via apply), and error states. Calling svc.execute_plan() on a plan in any of these states will raise InvalidPhaseTransitionError or BusinessRuleViolation from the domain layer, which gets mapped to a generic A2A error by map_domain_error(). The caller receives an opaque error instead of a clear "plan is in terminal state" response.

This was noted in the docstring for _handle_plan_execute: "If the plan has already reached (or passed) the execute phase, acknowledge idempotently" — but the implementation only handles 2 of the 5+ non-initial phases.

Evidence

# facade.py lines 357-363
plan = svc.get_plan(plan_id)
if plan.phase.value in ("execute", "apply"):  # Missing: cancelled, errored, completed
    return {"plan_id": plan.identity.plan_id, "status": plan.phase.value}
plan = svc.execute_plan(plan_id)   # Raises for terminal phases

Expected Behavior

Terminal phases (cancelled, errored, completed) should be included in the idempotency guard, returning the current status without attempting a phase transition.

Actual Behavior

Calling _cleveragents/plan/execute on a cancelled or errored plan raises an unhandled exception that becomes a generic A2A error response.

Suggested Fix

TERMINAL_PHASES = {"execute", "apply", "cancelled", "errored", "completed"}
if plan.phase.value in TERMINAL_PHASES:
    return {"plan_id": plan.identity.plan_id, "status": plan.phase.value}

TDD Note

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

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

## Bug Report: [spec-alignment] — A2aLocalFacade._handle_plan_execute() Phase Guard Uses Wrong Condition ### Severity Assessment - **Impact**: The phase check in `_handle_plan_execute()` skips execution if the plan is already in `execute` or `apply` phase. However it should also skip (or raise) if the plan is in a terminal phase (`cancelled`, `errored`, `completed`). As written, a cancelled or errored plan passed to `_cleveragents/plan/execute` will attempt `svc.execute_plan()` and raise an unhandled domain error that bubbles up as an unspecified A2A error code. - **Likelihood**: Medium — affects idempotency in automated retry scenarios. - **Priority**: Medium ### Location - **File**: `src/cleveragents/a2a/facade.py` - **Function/Class**: `A2aLocalFacade._handle_plan_execute` - **Lines**: 351–363 ### Description `_handle_plan_execute()` guards against duplicate transitions: ```python def _handle_plan_execute(self, params: dict[str, Any]) -> dict[str, Any]: svc = self._plan_lifecycle_service plan_id = params.get("plan_id", "") ... plan = svc.get_plan(plan_id) if plan.phase.value in ("execute", "apply"): # INCOMPLETE GUARD 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} ``` The guard only checks for `execute` and `apply` phases. The spec defines additional terminal phases: `cancelled`, `completed` (via apply), and error states. Calling `svc.execute_plan()` on a plan in any of these states will raise `InvalidPhaseTransitionError` or `BusinessRuleViolation` from the domain layer, which gets mapped to a generic A2A error by `map_domain_error()`. The caller receives an opaque error instead of a clear "plan is in terminal state" response. This was noted in the docstring for `_handle_plan_execute`: "If the plan has already reached (or passed) the execute phase, acknowledge idempotently" — but the implementation only handles 2 of the 5+ non-initial phases. ### Evidence ```python # facade.py lines 357-363 plan = svc.get_plan(plan_id) if plan.phase.value in ("execute", "apply"): # Missing: cancelled, errored, completed return {"plan_id": plan.identity.plan_id, "status": plan.phase.value} plan = svc.execute_plan(plan_id) # Raises for terminal phases ``` ### Expected Behavior Terminal phases (`cancelled`, `errored`, `completed`) should be included in the idempotency guard, returning the current status without attempting a phase transition. ### Actual Behavior Calling `_cleveragents/plan/execute` on a cancelled or errored plan raises an unhandled exception that becomes a generic A2A error response. ### Suggested Fix ```python TERMINAL_PHASES = {"execute", "apply", "cancelled", "errored", "completed"} if plan.phase.value in TERMINAL_PHASES: return {"plan_id": plan.identity.plan_id, "status": plan.phase.value} ``` ### Category spec-alignment ### TDD Note After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD with `@tdd_expected_fail` tags. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor

HAL9000 added the

Type

Bug

label

2026-04-10 21:36:56 +00:00

HAL9000 referenced this issue

2026-04-10 21:37:11 +00:00

[AUTO-BUG-SUP] Bug Hunt Pool Status (Cycle 1) #7531

HAL9000 added the

labels

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

HAL9000 added this to the v3.5.0 milestone

2026-04-10 23:05:50 +00:00

HAL9000 commented

2026-04-10 23:11:17 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: Backlog — Minor bug or optimization that does not block milestone delivery
Milestone: Assigned to appropriate milestone for future work
Story Points: 2 (S) — Small fix
MoSCoW: Could Have — Nice to fix but not blocking

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

Issue triaged by project owner: - **State**: Verified - **Priority**: Backlog — Minor bug or optimization that does not block milestone delivery - **Milestone**: Assigned to appropriate milestone for future work - **Story Points**: 2 (S) — Small fix - **MoSCoW**: Could Have — Nice to fix but not blocking --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 referenced this issue

2026-04-10 23:30:28 +00:00

[AUTO-OWNR] Project Owner Status (Cycle 4) #7585

HAL9000 referenced this issue

2026-04-11 02:41:45 +00:00

[AUTO-ARCH] Architecture Status (Cycle 4) #7683

HAL9000 referenced this issue

2026-04-11 13:16:33 +00:00

[AUTO-ARCH] Architecture Status (Cycle 4) #7683

freemo added the

Priority

Backlog

label

2026-04-12 18:34:03 +00:00

HAL9000 commented

2026-04-13 05:08:35 +00:00

Author

Owner

Implementation Attempt Starting — Tier 1: haiku — [AUTO-IMP-ISSUE-7529]

Beginning implementation of the phase transition guard fix for A2aLocalFacade._handle_plan_execute().

Plan:

Fix the incomplete phase guard in src/cleveragents/a2a/facade.py to include terminal phases (cancelled, errored, completed)
Add BDD tests covering the terminal phase scenarios
Run all quality gates (lint, typecheck, unit tests, integration tests)
Create PR closing this issue

Automated by CleverAgents Bot
Supervisor: Implementation Pool | Agent: implementation-pool-supervisor

**Implementation Attempt Starting** — Tier 1: haiku — [AUTO-IMP-ISSUE-7529] Beginning implementation of the phase transition guard fix for `A2aLocalFacade._handle_plan_execute()`. **Plan:** 1. Fix the incomplete phase guard in `src/cleveragents/a2a/facade.py` to include terminal phases (`cancelled`, `errored`, `completed`) 2. Add BDD tests covering the terminal phase scenarios 3. Run all quality gates (lint, typecheck, unit tests, integration tests) 4. Create PR closing this issue --- **Automated by CleverAgents Bot** Supervisor: Implementation Pool | Agent: implementation-pool-supervisor

HAL9000 referenced this issue from a commit

2026-04-13 06:21:33 +00:00

fix(a2a): extend _handle_plan_execute phase guard to include terminal phases

HAL9000 referenced this issue from a pull request that will close it,

2026-04-13 06:23:52 +00:00

fix(a2a): extend _handle_plan_execute phase guard to include terminal phases #8252

HAL9000 removed the

State

Verified

label

2026-04-13 06:30:57 +00:00

HAL9000 commented

2026-04-13 06:33:28 +00:00

Author

Owner

Implementation Attempt — Tier 1: haiku — Success [AUTO-IMP-ISSUE-7529]

What was done

Fixed the incomplete phase transition guard in A2aLocalFacade._handle_plan_execute() in src/cleveragents/a2a/facade.py.

Root cause: The method only guarded against in-progress phases (execute, apply) but not terminal phases (cancelled, errored, completed). Calling _cleveragents/plan/execute on a plan in a terminal phase would attempt a phase transition and raise an unhandled InvalidPhaseTransitionError or BusinessRuleViolation.

Fix:

Added class-level constant _EXECUTE_GUARD_PHASES as a frozenset containing all phases that should return idempotently: {"execute", "apply", "cancelled", "errored", "completed"}
Updated the guard condition to check against this frozenset

Tests added:

features/a2a_facade_terminal_phase_guard.feature — 10 BDD scenarios covering all terminal and in-progress phase cases
features/steps/a2a_facade_terminal_phase_guard_steps.py — step definitions

Quality gates

✅ nox -e lint — passed
✅ nox -e typecheck — passed (0 errors)
✅ nox -e unit_tests — 10 new scenarios pass, all existing a2a facade tests pass

PR

Created PR #8252: #8252

Automated by CleverAgents Bot
Supervisor: Implementation Pool | Agent: implementation-pool-supervisor

**Implementation Attempt** — Tier 1: haiku — Success [AUTO-IMP-ISSUE-7529] ## What was done Fixed the incomplete phase transition guard in `A2aLocalFacade._handle_plan_execute()` in `src/cleveragents/a2a/facade.py`. **Root cause**: The method only guarded against in-progress phases (`execute`, `apply`) but not terminal phases (`cancelled`, `errored`, `completed`). Calling `_cleveragents/plan/execute` on a plan in a terminal phase would attempt a phase transition and raise an unhandled `InvalidPhaseTransitionError` or `BusinessRuleViolation`. **Fix**: - Added class-level constant `_EXECUTE_GUARD_PHASES` as a `frozenset` containing all phases that should return idempotently: `{"execute", "apply", "cancelled", "errored", "completed"}` - Updated the guard condition to check against this frozenset **Tests added**: - `features/a2a_facade_terminal_phase_guard.feature` — 10 BDD scenarios covering all terminal and in-progress phase cases - `features/steps/a2a_facade_terminal_phase_guard_steps.py` — step definitions ## Quality gates - ✅ `nox -e lint` — passed - ✅ `nox -e typecheck` — passed (0 errors) - ✅ `nox -e unit_tests` — 10 new scenarios pass, all existing a2a facade tests pass ## PR Created PR #8252: https://git.cleverthis.com/cleveragents/cleveragents-core/pulls/8252 --- **Automated by CleverAgents Bot** Supervisor: Implementation Pool | Agent: implementation-pool-supervisor