From 4015698e95a3727fbe06862788ec525f87bcc1f5 Mon Sep 17 00:00:00 2001 From: CleverThis Date: Fri, 10 Apr 2026 19:05:04 +0000 Subject: [PATCH] docs(spec): clarify fail_fast cancel semantics and A2A facade idempotency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two spec gaps identified from UAT and bug hunt findings: 1. fail_fast cancel semantics (UAT #7394): The spec defined fail_fast as a SubplanConfig option but did not specify what state unstarted subplans should transition to when fail_fast triggers. UAT found that unstarted subplans end up in 'complete' state instead of 'cancelled', which is a spec violation. Clarification: When fail_fast=true and a parallel subplan errors, all queued (not yet started) subplans MUST transition to 'cancelled' state. 'cancelled' is semantically distinct from 'complete' — it means the subplan was never started due to a sibling failure. 2. A2A facade idempotency contract (bug #7389): The spec described A2A extension methods but did not specify that they must be idempotent. Bug found that _handle_plan_apply() raises InvalidPhaseTransitionError when called on an already-applied plan, breaking idempotency. Clarification: All _cleveragents/plan/* methods MUST be idempotent. Before invoking the underlying service, check the current plan phase. If already at or past the target phase, return current state without re-invoking. Raising InvalidPhaseTransitionError to the A2A caller is a spec violation. Refs: UAT #7394, bug #7389 --- docs/specification.md | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/docs/specification.md b/docs/specification.md index dbe7bd381..24dd5de1f 100644 --- a/docs/specification.md +++ b/docs/specification.md @@ -18508,6 +18508,14 @@ The `local/plan-tools` skill references this tool (and others) by name: Parallel execution is bounded by `SubplanConfig.max_parallel` (default: `5`, range: 1–50). This cap prevents runaway resource consumption when a large number of child plans are spawned simultaneously. The runtime uses a `ThreadPoolExecutor` with `min(max_parallel, len(subplans))` workers. The `SubplanConfig` model also controls `merge_strategy` (default: `git_three_way`), `fail_fast` (default: `false`), `timeout_per_subplan_seconds` (default: `null`), `retry_failed` (default: `true`), and `max_retries` (default: `2`). + **`fail_fast` semantics**: When `fail_fast: true` and any parallel subplan transitions to `errored`, the runtime MUST: + 1. Stop dispatching new subplans from the queue (no new futures submitted to the thread pool) + 2. Transition all **queued** (not yet started) subplans to `cancelled` state — NOT `complete` + 3. Allow already-running subplans to finish naturally (they are not forcibly interrupted) + 4. After all running subplans complete, proceed with failure handling for the parent plan + + The `cancelled` state is semantically distinct from `complete`: `cancelled` means the subplan was never started due to a sibling failure, while `complete` means the subplan ran to completion. Downstream tooling (plan inspectors, orchestrators, CI systems) relies on this distinction to determine whether work was skipped vs. completed. Transitioning unstarted subplans to `complete` when `fail_fast` triggers is a **spec violation**. + === "Dependency-Ordered" When child plans have explicit dependencies on each other, the `DEPENDENCY_ORDERED` execution mode respects those dependencies while maximizing concurrency. The dependency graph is provided as a `dependency_graph: dict[str, list[str]]` mapping each `subplan_id` to the list of `subplan_ids` it depends on. @@ -43470,6 +43478,36 @@ Where `{entity}` is one of: `actor`, `skill`, `tool`, `validation`, `resource`, | `_cleveragents/health/check` | Health check handler | | `_cleveragents/diagnostics/run` | Diagnostic runner | +##### A2A Facade Idempotency Contract + +All `_cleveragents/plan/*` extension methods handled by `A2aLocalFacade` MUST be **idempotent** — calling the same method twice with the same parameters must produce the same result without raising an error. This is required because: + +1. Network retries and orchestrators may call the same method multiple times +2. The A2A protocol does not guarantee exactly-once delivery +3. CLI commands may be retried by users or automation scripts + +**Idempotency implementation pattern**: Before invoking the underlying service method, check the current plan phase. If the plan has already reached or passed the target phase, return the current state without re-invoking: + +```python +def _handle_plan_execute(self, params: dict) -> dict: + plan = svc.get_plan(plan_id) + # Idempotency guard: if already at or past execute phase, acknowledge + 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) -> dict: + plan = svc.get_plan(plan_id) + # Idempotency guard: if already applied, acknowledge without re-applying + 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} +``` + +Raising `InvalidPhaseTransitionError` to the A2A caller when the plan is already in the target phase is a **spec violation** — it breaks idempotency and causes orchestrators to treat a successful (already-applied) plan as a failure. + ##### Streaming Architecture A2A streaming uses **Server-Sent Events (SSE)** via `message/stream`. The server pushes `TaskStatusUpdateEvent` and `TaskArtifactUpdateEvent` to the client in real-time during long-running operations: -- 2.52.0