bug(cli): plan execute only transitions state — does not run strategize or execute phase processing #967

Closed
opened 2026-03-16 03:16:18 +00:00 by freemo · 2 comments
Owner

Metadata

  • Commit Message: fix(cli): restore phase-aware execution in plan execute command
  • Branch: bugfix/m3-plan-execute-phase-processing

Background and Context

The M3 (v3.2.0) acceptance test (robot/e2e/m3_acceptance.robot, PR #799) exercises the full plan lifecycle: plan useplan execute (strategize) → plan execute (execute) → plan correctplan lifecycle-apply. The plan execute CLI command is expected to detect the plan's current phase and run the appropriate processing:

  • Strategize/QUEUED: run the strategize phase via PlanExecutor.run_strategize() to produce decisions
  • Strategize/COMPLETE: transition to Execute phase and run execute via PlanExecutor.run_execute()
  • Execute/QUEUED: run the execute phase via PlanExecutor.run_execute()

Current Behavior

The execute_plan function in src/cleveragents/cli/commands/plan.py (line ~1499) only calls service.execute_plan(plan_id), which is a state transition only (Strategize/COMPLETE → Execute/QUEUED). It does not:

  1. Construct a PlanExecutor instance
  2. Call PlanExecutor.run_strategize() when the plan is in Strategize/QUEUED
  3. Call PlanExecutor.run_execute() when the plan transitions to or is in Execute/QUEUED

After plan use, the plan is in Strategize/QUEUED state. The first plan execute call fails immediately because service.execute_plan() requires the plan to be in Strategize/COMPLETE state — it raises PlanNotReadyError, the CLI catches it and aborts with rc=1.

Additionally, the _get_plan_executor() helper function was removed from plan.py, so there is no way to construct a PlanExecutor from the CLI layer.

The auto-discovery path (when no plan_id is given) also fails because it only searches for plans in Strategize/COMPLETE state, missing plans in QUEUED state that need strategize processing.

Expected Behavior

  1. plan execute PLAN_ID detects the plan's current phase/state and runs the appropriate processing
  2. When the plan is in Strategize/QUEUED, PlanExecutor.run_strategize() is called to process the strategize phase to completion
  3. When the plan is in Strategize/COMPLETE, service.execute_plan() transitions to Execute, then PlanExecutor.run_execute() processes the execute phase
  4. When the plan is in Execute/QUEUED, PlanExecutor.run_execute() processes the execute phase
  5. Auto-discovery (no plan_id) finds plans in both QUEUED and COMPLETE states across Strategize and Execute phases
  6. The command outputs the plan status after processing, using the requested format

Acceptance Criteria

  • _get_plan_executor() helper is restored in plan.py to construct a PlanExecutor(lifecycle_service=service)
  • plan execute handles Strategize/QUEUED by running executor.run_strategize(plan_id)
  • plan execute handles Strategize/COMPLETE by transitioning to Execute and running executor.run_execute(plan_id)
  • plan execute handles Execute/QUEUED by running executor.run_execute(plan_id)
  • Auto-discovery includes plans in Strategize/QUEUED and Execute/QUEUED states
  • The M3 E2E acceptance test steps 7 and 10 (plan execute) pass without Traceback
  • Tests (Behave): Existing plan execute scenarios continue to pass
  • Tests (Robot): M3 acceptance test plan execute steps pass
  • Verify coverage >=97% via nox -s coverage_report
  • Run nox (all default sessions), fix any errors

Subtasks

  • Add _get_plan_executor() helper function that constructs PlanExecutor(lifecycle_service=service)
  • Rewrite execute_plan CLI to detect plan phase/state and dispatch to appropriate executor method
  • Update auto-discovery to include QUEUED plans in both Strategize and Execute phases
  • Tests (Behave): Verify existing plan execute scenarios pass
  • Tests (Robot): Verify M3 acceptance test steps 7 and 10 pass
  • Verify coverage >=97% via nox -s coverage_report
  • Run nox (all default sessions), fix any errors

Definition of Done

This issue is complete when:

  • All subtasks above are completed and checked off.
  • A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation.
  • The commit is pushed to the remote on the branch matching the Branch in Metadata exactly.
  • The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.
## Metadata - **Commit Message**: `fix(cli): restore phase-aware execution in plan execute command` - **Branch**: `bugfix/m3-plan-execute-phase-processing` ## Background and Context The M3 (v3.2.0) acceptance test (`robot/e2e/m3_acceptance.robot`, PR #799) exercises the full plan lifecycle: `plan use` → `plan execute` (strategize) → `plan execute` (execute) → `plan correct` → `plan lifecycle-apply`. The `plan execute` CLI command is expected to detect the plan's current phase and run the appropriate processing: - **Strategize/QUEUED**: run the strategize phase via `PlanExecutor.run_strategize()` to produce decisions - **Strategize/COMPLETE**: transition to Execute phase and run execute via `PlanExecutor.run_execute()` - **Execute/QUEUED**: run the execute phase via `PlanExecutor.run_execute()` ## Current Behavior The `execute_plan` function in `src/cleveragents/cli/commands/plan.py` (line ~1499) only calls `service.execute_plan(plan_id)`, which is a **state transition only** (Strategize/COMPLETE → Execute/QUEUED). It does not: 1. Construct a `PlanExecutor` instance 2. Call `PlanExecutor.run_strategize()` when the plan is in Strategize/QUEUED 3. Call `PlanExecutor.run_execute()` when the plan transitions to or is in Execute/QUEUED After `plan use`, the plan is in Strategize/QUEUED state. The first `plan execute` call fails immediately because `service.execute_plan()` requires the plan to be in Strategize/COMPLETE state — it raises `PlanNotReadyError`, the CLI catches it and aborts with rc=1. Additionally, the `_get_plan_executor()` helper function was removed from `plan.py`, so there is no way to construct a `PlanExecutor` from the CLI layer. The auto-discovery path (when no plan_id is given) also fails because it only searches for plans in Strategize/COMPLETE state, missing plans in QUEUED state that need strategize processing. ## Expected Behavior 1. `plan execute PLAN_ID` detects the plan's current phase/state and runs the appropriate processing 2. When the plan is in Strategize/QUEUED, `PlanExecutor.run_strategize()` is called to process the strategize phase to completion 3. When the plan is in Strategize/COMPLETE, `service.execute_plan()` transitions to Execute, then `PlanExecutor.run_execute()` processes the execute phase 4. When the plan is in Execute/QUEUED, `PlanExecutor.run_execute()` processes the execute phase 5. Auto-discovery (no plan_id) finds plans in both QUEUED and COMPLETE states across Strategize and Execute phases 6. The command outputs the plan status after processing, using the requested format ## Acceptance Criteria - [ ] `_get_plan_executor()` helper is restored in `plan.py` to construct a `PlanExecutor(lifecycle_service=service)` - [ ] `plan execute` handles Strategize/QUEUED by running `executor.run_strategize(plan_id)` - [ ] `plan execute` handles Strategize/COMPLETE by transitioning to Execute and running `executor.run_execute(plan_id)` - [ ] `plan execute` handles Execute/QUEUED by running `executor.run_execute(plan_id)` - [ ] Auto-discovery includes plans in Strategize/QUEUED and Execute/QUEUED states - [ ] The M3 E2E acceptance test steps 7 and 10 (`plan execute`) pass without Traceback - [ ] Tests (Behave): Existing plan execute scenarios continue to pass - [ ] Tests (Robot): M3 acceptance test `plan execute` steps pass - [ ] Verify coverage >=97% via `nox -s coverage_report` - [ ] Run `nox` (all default sessions), fix any errors ## Subtasks - [ ] Add `_get_plan_executor()` helper function that constructs `PlanExecutor(lifecycle_service=service)` - [ ] Rewrite `execute_plan` CLI to detect plan phase/state and dispatch to appropriate executor method - [ ] Update auto-discovery to include QUEUED plans in both Strategize and Execute phases - [ ] Tests (Behave): Verify existing plan execute scenarios pass - [ ] Tests (Robot): Verify M3 acceptance test steps 7 and 10 pass - [ ] Verify coverage >=97% via `nox -s coverage_report` - [ ] Run `nox` (all default sessions), fix any errors ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly. - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done.
freemo added this to the v3.2.0 milestone 2026-03-16 03:16:26 +00:00
Author
Owner

TDD workflow initiated for this bug:

  • Created TDD issue #977 to write a tagged test that captures this bug.
  • Dependency link needed: this issue is blocked by #977 (Forgejo dependency API is returning errors — a maintainer should manually add the dependency link via the UI: #967 depends on #977).
  • Once #977 is merged to master, @freemo should create branch bugfix/m3-plan-execute-phase-processing from master and implement the fix.
  • The fix must remove the @tdd_expected_fail tag from the test so it runs normally.
  • The CI quality gate will verify the tag was removed before the PR can be merged.

Assignees:

  • TDD issue #977: @brent.edwards (detail-oriented, good for precise tag work)
  • Bug fix (this issue): @freemo (plan lifecycle expert, author of the codebase)

TDD Workflow

This bug follows the project's mandatory TDD workflow for bug fixes:

  1. TDD issue #977 must be completed first — it creates a tagged test that captures this bug using the @tdd_expected_fail mechanism.
  2. Once the TDD PR is merged to master, @freemo creates branch bugfix/m3-plan-execute-phase-processing from master.
  3. The fix is implemented and the @tdd_expected_fail tag is removed from the test (leaving @tdd_bug and @tdd_bug_967 in place as permanent references).
  4. The test now runs normally and must pass, confirming the bug is fixed.
  5. The CI quality gate verifies that the @tdd_expected_fail tag has been removed.
**TDD workflow initiated for this bug:** - Created TDD issue #977 to write a tagged test that captures this bug. - Dependency link needed: this issue is blocked by #977 (Forgejo dependency API is returning errors — a maintainer should manually add the dependency link via the UI: #967 depends on #977). - Once #977 is merged to `master`, @freemo should create branch `bugfix/m3-plan-execute-phase-processing` from `master` and implement the fix. - The fix must remove the `@tdd_expected_fail` tag from the test so it runs normally. - The CI quality gate will verify the tag was removed before the PR can be merged. **Assignees:** - TDD issue #977: @brent.edwards (detail-oriented, good for precise tag work) - Bug fix (this issue): @freemo (plan lifecycle expert, author of the codebase) ### TDD Workflow This bug follows the project's mandatory TDD workflow for bug fixes: 1. TDD issue #977 must be completed first — it creates a tagged test that captures this bug using the `@tdd_expected_fail` mechanism. 2. Once the TDD PR is merged to `master`, @freemo creates branch `bugfix/m3-plan-execute-phase-processing` from `master`. 3. The fix is implemented and the `@tdd_expected_fail` tag is removed from the test (leaving `@tdd_bug` and `@tdd_bug_967` in place as permanent references). 4. The test now runs normally and must pass, confirming the bug is fixed. 5. The CI quality gate verifies that the `@tdd_expected_fail` tag has been removed.
Member

Implementation Notes — #967

Branch: bugfix/m3-plan-execute-phase-processing | Commit: 37d8ac96 | PR: #1056

Design Decisions

The execute_plan CLI command on master was already functionally correct — the core phase-aware dispatch (_get_plan_executor(), run_strategize()/run_execute(), QUEUED-inclusive auto-discovery) had been applied across earlier commits. This fix consolidates and polishes:

  1. Eliminated redundant service.get_plan() call — the separate pre_plan fetch for the read-only guard was consolidated into the single current_plan fetch, removing one unnecessary persistence round-trip.

  2. Updated stale command-table description — "Transition to Execute phase" → "Run phase-aware plan execution" to accurately reflect actual behavior.

  3. Improved post-execution status message — replaced static hint with state-aware message showing actual phase/state and appropriate next-step guidance.

  4. Updated 3 Behave test files to match the reduced get_plan call sequence and updated output assertions.

Quality Gates

  • Lint: PASS
  • Typecheck: PASS (0 errors)
  • Unit tests: All plan-related scenarios pass (293 across 7 feature files)
## Implementation Notes — #967 **Branch:** `bugfix/m3-plan-execute-phase-processing` | **Commit:** `37d8ac96` | **PR:** #1056 ### Design Decisions The `execute_plan` CLI command on master was already functionally correct — the core phase-aware dispatch (`_get_plan_executor()`, `run_strategize()`/`run_execute()`, QUEUED-inclusive auto-discovery) had been applied across earlier commits. This fix consolidates and polishes: 1. **Eliminated redundant `service.get_plan()` call** — the separate `pre_plan` fetch for the read-only guard was consolidated into the single `current_plan` fetch, removing one unnecessary persistence round-trip. 2. **Updated stale command-table description** — "Transition to Execute phase" → "Run phase-aware plan execution" to accurately reflect actual behavior. 3. **Improved post-execution status message** — replaced static hint with state-aware message showing actual `phase/state` and appropriate next-step guidance. 4. **Updated 3 Behave test files** to match the reduced `get_plan` call sequence and updated output assertions. ### Quality Gates - Lint: PASS - Typecheck: PASS (0 errors) - Unit tests: All plan-related scenarios pass (293 across 7 feature files)
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#967
No description provided.