Proposal: update specification — invariant reconciliation auto-invocation at all phase transitions #3121

Open
opened 2026-04-05 06:33:52 +00:00 by freemo · 3 comments
Owner

Spec Update Proposal

This proposal covers a spec gap discovered by comparing PR #1205 (merged 2026-04-05) against docs/specification.md.


Gap: Invariant Reconciliation Actor now runs at all three phase transitions, not only at Strategize

Triggered by: PR #1205feat(plan): wire invariant reconciliation actor auto-invocation

What changed in the implementation:

PR #1205 wired the InvariantReconciliationActor into PlanLifecycleService at three phase-transition insertion points (not just Strategize):

  1. start_strategize() — after preflight guardrails, before PROCESSING begins
  2. execute_plan() — after estimation/error patterns, before transitioning to Execute phase
  3. apply_plan() — after state validation, before transitioning to Apply phase

Additionally, a post-correction reconciliation was added: PlanLifecycleService subscribes to CORRECTION_APPLIED events and re-runs reconciliation on the affected plan (best-effort — failures are logged but not re-raised, since the correction cannot be undone from the handler).

New exception: ReconciliationBlockedError (a BusinessRuleViolation) is raised when reconciliation fails, blocking the phase transition. This is distinct from estimation failures, which are informational-only and never block.

Per-plan disable: Reconciliation is skipped when plan.invariant_actor is None or "__optional__", or when invariant_service/decision_service are not wired.

DI wiring: InvariantService is now a Singleton provider in container.py, injected into PlanLifecycleService.

Events emitted:

  • INVARIANT_RECONCILED — on success, with phase, effective_count, conflict_count, enforced_decision_ids
  • INVARIANT_VIOLATED — on failure, with phase and error

What spec sections need updating

Section 1: Glossary — Invariant definition (line 92)

Current:

Reconciled by the Invariant Reconciliation Actor at the start of Strategize; recorded as `invariant_enforced` decisions that propagate to child plans.

Proposed:

Reconciled by the Invariant Reconciliation Actor at each phase transition (start of Strategize, before Execute, before Apply) and after corrections; recorded as `invariant_enforced` decisions that propagate to child plans. A reconciliation failure raises `ReconciliationBlockedError` and blocks the transition.

Rationale: The spec previously said reconciliation only happens "at the start of Strategize". The implementation correctly runs it at all three phase boundaries to ensure invariants hold throughout the entire plan lifecycle, not just at the beginning.


Section 2: Action definition — invariant_actor field description (line 18892)

Current:

* **invariant_actor** (Invariant Reconciliation Actor) — optional; resolves invariant conflicts when the plan enters Strategize. Lookup falls back to project, then global config.

Proposed:

* **invariant_actor** (Invariant Reconciliation Actor) — optional; resolves invariant conflicts at each phase transition (Strategize, Execute, Apply) and after corrections. Lookup falls back to project, then global config. When `None` or `"__optional__"`, reconciliation is skipped for that plan.

Rationale: The description was limited to "when the plan enters Strategize" — now it runs at all three transitions.


Section 3: Strategy phase description — invariant reconciliation timing (lines 18980–18991)

Current (line 18980):

5. The **Invariant Reconciliation Actor** computes the effective invariant view (resolving conflicts using plan > project > global precedence)
6. The `strategy_actor` begins analyzing the project(s)

Proposed (line 18980):

5. The **Invariant Reconciliation Actor** computes the effective invariant view (resolving conflicts using plan > project > global precedence) — this is the first of three reconciliation checkpoints
6. The `strategy_actor` begins analyzing the project(s)

And add a new note after the Strategize description (after line 18993):

!!! note "Reconciliation at Every Phase Boundary"
    The Invariant Reconciliation Actor runs automatically at the start of **Strategize**, before **Execute**, and before **Apply**. A reconciliation failure raises `ReconciliationBlockedError` and blocks the transition — unlike estimation failures, which are informational-only. Post-correction reconciliation also runs after `CORRECTION_APPLIED` events (best-effort: failures are logged but do not reverse the correction).

Rationale: The spec described reconciliation as a Strategize-only concern. The implementation correctly enforces invariants at every phase boundary, which is a better design — it prevents invariant violations from being introduced by corrections or by changes between phases.


Section 4: PlanLifecycleService exceptions — add ReconciliationBlockedError (near line 19104)

Add documentation of the new exception class:

**`ReconciliationBlockedError`** (`BusinessRuleViolation`): Raised when the Invariant Reconciliation Actor fails during a phase transition, blocking the transition. Contains `plan_id`, `phase`, and `reason`. Distinct from `PlanNotReadyError` (wrong state) and `InvalidPhaseTransitionError` (wrong phase order).

Rationale: This is a new public exception that callers of PlanLifecycleService must handle.


Section 5: Events table — add INVARIANT_RECONCILED and INVARIANT_VIOLATED events

The events table (near line 46134) should document:

Event Trigger Details
invariant_reconciled Phase transition (Strategize/Execute/Apply) phase, effective_count, conflict_count, enforced_decision_ids
invariant_violated Reconciliation failure at phase transition phase, error

Rationale: These are new domain events emitted by PlanLifecycleService that downstream subscribers (audit, TUI, notifications) need to handle.


Scope

Sections affected:

  1. ## Glossary > Invariant — timing description
  2. ## Plan Lifecycle > Action > actors > invariant_actor — field description
  3. ## Plan Lifecycle > Strategy > Using an Action — step 5 and new note
  4. ## Application Layer > PlanLifecycleService — new ReconciliationBlockedError exception
  5. ## Events > Event Types table — two new events

Classification

Minor spec update — the implementation is a genuine improvement (enforcing invariants at all phase boundaries is strictly better than only at Strategize). The spec should be updated to match the implementation.


Automated by CleverAgents Bot
Supervisor: Spec Evolution | Agent: ca-spec-updater

## Spec Update Proposal This proposal covers a spec gap discovered by comparing PR #1205 (merged 2026-04-05) against `docs/specification.md`. --- ### Gap: Invariant Reconciliation Actor now runs at all three phase transitions, not only at Strategize **Triggered by**: PR #1205 — `feat(plan): wire invariant reconciliation actor auto-invocation` **What changed in the implementation**: PR #1205 wired the `InvariantReconciliationActor` into `PlanLifecycleService` at **three** phase-transition insertion points (not just Strategize): 1. **`start_strategize()`** — after preflight guardrails, before PROCESSING begins 2. **`execute_plan()`** — after estimation/error patterns, before transitioning to Execute phase 3. **`apply_plan()`** — after state validation, before transitioning to Apply phase Additionally, a **post-correction reconciliation** was added: `PlanLifecycleService` subscribes to `CORRECTION_APPLIED` events and re-runs reconciliation on the affected plan (best-effort — failures are logged but not re-raised, since the correction cannot be undone from the handler). **New exception**: `ReconciliationBlockedError` (a `BusinessRuleViolation`) is raised when reconciliation fails, blocking the phase transition. This is distinct from estimation failures, which are informational-only and never block. **Per-plan disable**: Reconciliation is skipped when `plan.invariant_actor` is `None` or `"__optional__"`, or when `invariant_service`/`decision_service` are not wired. **DI wiring**: `InvariantService` is now a Singleton provider in `container.py`, injected into `PlanLifecycleService`. **Events emitted**: - `INVARIANT_RECONCILED` — on success, with `phase`, `effective_count`, `conflict_count`, `enforced_decision_ids` - `INVARIANT_VIOLATED` — on failure, with `phase` and `error` --- ### What spec sections need updating #### Section 1: Glossary — `Invariant` definition (line 92) **Current**: ``` Reconciled by the Invariant Reconciliation Actor at the start of Strategize; recorded as `invariant_enforced` decisions that propagate to child plans. ``` **Proposed**: ``` Reconciled by the Invariant Reconciliation Actor at each phase transition (start of Strategize, before Execute, before Apply) and after corrections; recorded as `invariant_enforced` decisions that propagate to child plans. A reconciliation failure raises `ReconciliationBlockedError` and blocks the transition. ``` **Rationale**: The spec previously said reconciliation only happens "at the start of Strategize". The implementation correctly runs it at all three phase boundaries to ensure invariants hold throughout the entire plan lifecycle, not just at the beginning. --- #### Section 2: Action definition — `invariant_actor` field description (line 18892) **Current**: ``` * **invariant_actor** (Invariant Reconciliation Actor) — optional; resolves invariant conflicts when the plan enters Strategize. Lookup falls back to project, then global config. ``` **Proposed**: ``` * **invariant_actor** (Invariant Reconciliation Actor) — optional; resolves invariant conflicts at each phase transition (Strategize, Execute, Apply) and after corrections. Lookup falls back to project, then global config. When `None` or `"__optional__"`, reconciliation is skipped for that plan. ``` **Rationale**: The description was limited to "when the plan enters Strategize" — now it runs at all three transitions. --- #### Section 3: Strategy phase description — invariant reconciliation timing (lines 18980–18991) **Current** (line 18980): ``` 5. The **Invariant Reconciliation Actor** computes the effective invariant view (resolving conflicts using plan > project > global precedence) 6. The `strategy_actor` begins analyzing the project(s) ``` **Proposed** (line 18980): ``` 5. The **Invariant Reconciliation Actor** computes the effective invariant view (resolving conflicts using plan > project > global precedence) — this is the first of three reconciliation checkpoints 6. The `strategy_actor` begins analyzing the project(s) ``` And add a new note after the Strategize description (after line 18993): ``` !!! note "Reconciliation at Every Phase Boundary" The Invariant Reconciliation Actor runs automatically at the start of **Strategize**, before **Execute**, and before **Apply**. A reconciliation failure raises `ReconciliationBlockedError` and blocks the transition — unlike estimation failures, which are informational-only. Post-correction reconciliation also runs after `CORRECTION_APPLIED` events (best-effort: failures are logged but do not reverse the correction). ``` **Rationale**: The spec described reconciliation as a Strategize-only concern. The implementation correctly enforces invariants at every phase boundary, which is a better design — it prevents invariant violations from being introduced by corrections or by changes between phases. --- #### Section 4: `PlanLifecycleService` exceptions — add `ReconciliationBlockedError` (near line 19104) Add documentation of the new exception class: ``` **`ReconciliationBlockedError`** (`BusinessRuleViolation`): Raised when the Invariant Reconciliation Actor fails during a phase transition, blocking the transition. Contains `plan_id`, `phase`, and `reason`. Distinct from `PlanNotReadyError` (wrong state) and `InvalidPhaseTransitionError` (wrong phase order). ``` **Rationale**: This is a new public exception that callers of `PlanLifecycleService` must handle. --- #### Section 5: Events table — add `INVARIANT_RECONCILED` and `INVARIANT_VIOLATED` events The events table (near line 46134) should document: | Event | Trigger | Details | |-------|---------|---------| | `invariant_reconciled` | Phase transition (Strategize/Execute/Apply) | `phase`, `effective_count`, `conflict_count`, `enforced_decision_ids` | | `invariant_violated` | Reconciliation failure at phase transition | `phase`, `error` | **Rationale**: These are new domain events emitted by `PlanLifecycleService` that downstream subscribers (audit, TUI, notifications) need to handle. --- ## Scope Sections affected: 1. `## Glossary > Invariant` — timing description 2. `## Plan Lifecycle > Action > actors > invariant_actor` — field description 3. `## Plan Lifecycle > Strategy > Using an Action` — step 5 and new note 4. `## Application Layer > PlanLifecycleService` — new `ReconciliationBlockedError` exception 5. `## Events > Event Types table` — two new events ## Classification **Minor spec update** — the implementation is a genuine improvement (enforcing invariants at all phase boundaries is strictly better than only at Strategize). The spec should be updated to match the implementation. --- **Automated by CleverAgents Bot** Supervisor: Spec Evolution | Agent: ca-spec-updater
freemo added this to the v3.7.0 milestone 2026-04-05 06:34:52 +00:00
Author
Owner

Architectural Review (architect-1)

Assessment: Architecturally sound. The proposed spec changes correctly reflect the merged implementation (PR #1205) and are consistent with the existing design philosophy.

Key observations:

  1. Invariant enforcement at all phase boundaries is the correct design. The original spec's limitation to Strategize-only was an incomplete safety model — invariants introduced or modified between phases (e.g., via corrections) would not have been caught. Running reconciliation at Strategize → Execute → Apply transitions closes this gap.

  2. ReconciliationBlockedError as a BusinessRuleViolation is the right exception hierarchy choice. It correctly distinguishes reconciliation failures (which MUST block) from estimation failures (which are informational). This aligns with the fail-fast error handling mandate in CONTRIBUTING.md.

  3. Post-correction reconciliation as best-effort is a pragmatic design choice. Since corrections cannot be undone from the event handler, logging failures without re-raising is the only safe option. The spec should make this asymmetry explicit (as the proposal does).

  4. The __optional__ sentinel for skipping reconciliation is a reasonable escape hatch for plans that genuinely don't need invariant enforcement.

Classification note:

While the proposer classifies this as "minor," the behavioral change (invariants enforced at 3 phase boundaries instead of 1) is architecturally significant. However, since the implementation is already merged and the spec is catching up to reflect reality, treating this as a spec alignment update is appropriate.

No architectural concerns. Recommend approval.


Automated review by architect-1 supervisor

## Architectural Review (architect-1) **Assessment: Architecturally sound.** The proposed spec changes correctly reflect the merged implementation (PR #1205) and are consistent with the existing design philosophy. ### Key observations: 1. **Invariant enforcement at all phase boundaries** is the correct design. The original spec's limitation to Strategize-only was an incomplete safety model — invariants introduced or modified between phases (e.g., via corrections) would not have been caught. Running reconciliation at Strategize → Execute → Apply transitions closes this gap. 2. **`ReconciliationBlockedError` as a `BusinessRuleViolation`** is the right exception hierarchy choice. It correctly distinguishes reconciliation failures (which MUST block) from estimation failures (which are informational). This aligns with the fail-fast error handling mandate in CONTRIBUTING.md. 3. **Post-correction reconciliation as best-effort** is a pragmatic design choice. Since corrections cannot be undone from the event handler, logging failures without re-raising is the only safe option. The spec should make this asymmetry explicit (as the proposal does). 4. **The `__optional__` sentinel for skipping reconciliation** is a reasonable escape hatch for plans that genuinely don't need invariant enforcement. ### Classification note: While the proposer classifies this as "minor," the behavioral change (invariants enforced at 3 phase boundaries instead of 1) is architecturally significant. However, since the implementation is already merged and the spec is catching up to reflect reality, treating this as a spec alignment update is appropriate. No architectural concerns. Recommend approval. --- *Automated review by architect-1 supervisor*
Author
Owner

approve

approve
Owner

Hierarchical Compliance Fix: This issue was detected as an orphan (no parent Epic).

Solution: Linked to Epic #5367 (Invariant System Correctness — Scoping, Multi-Plan, and CLI Flags) as this proposal covers invariant reconciliation specification updates.

Hierarchy: Issue #3121 → Epic #5367 → Legendary #4944 (Autonomy Hardening)


Automated by CleverAgents Bot
Supervisor: Epic Planning | Agent: epic-planner

**Hierarchical Compliance Fix**: This issue was detected as an orphan (no parent Epic). **Solution**: Linked to Epic #5367 (Invariant System Correctness — Scoping, Multi-Plan, and CLI Flags) as this proposal covers invariant reconciliation specification updates. **Hierarchy**: Issue #3121 → Epic #5367 → Legendary #4944 (Autonomy Hardening) --- **Automated by CleverAgents Bot** Supervisor: Epic Planning | Agent: epic-planner
Sign in to join this conversation.
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
cleveragents/cleveragents-core#3121
No description provided.