fix(cli): restore phase-aware execution in plan execute command
CI / lint (pull_request) Successful in 14s
CI / typecheck (pull_request) Successful in 37s
CI / security (pull_request) Successful in 37s
CI / benchmark-publish (pull_request) Has been skipped
CI / quality (pull_request) Successful in 41s
CI / build (pull_request) Successful in 22s
CI / integration_tests (pull_request) Successful in 2m44s
CI / unit_tests (pull_request) Successful in 3m44s
CI / docker (pull_request) Successful in 39s
CI / coverage (pull_request) Successful in 5m3s
CI / e2e_tests (pull_request) Successful in 5m28s
CI / benchmark-regression (pull_request) Failing after 22m0s
CI / lint (pull_request) Successful in 14s
CI / typecheck (pull_request) Successful in 37s
CI / security (pull_request) Successful in 37s
CI / benchmark-publish (pull_request) Has been skipped
CI / quality (pull_request) Successful in 41s
CI / build (pull_request) Successful in 22s
CI / integration_tests (pull_request) Successful in 2m44s
CI / unit_tests (pull_request) Successful in 3m44s
CI / docker (pull_request) Successful in 39s
CI / coverage (pull_request) Successful in 5m3s
CI / e2e_tests (pull_request) Successful in 5m28s
CI / benchmark-regression (pull_request) Failing after 22m0s
Consolidate the execute_plan CLI handler to eliminate a redundant service.get_plan() call (the separate read-only pre-check now reuses the same current_plan reference used for phase detection), update the command-table description from the stale 'Transition to Execute phase' to 'Run phase-aware plan execution', and replace the static post-execution hint with a state-aware message that distinguishes execute/complete (ready for apply) from other states (continue executing). Update corresponding Behave test mocks to match the reduced get_plan call sequence and the updated output panel title. ISSUES CLOSED: #967
This commit is contained in:
@@ -47,7 +47,7 @@ Feature: Plan lifecycle CLI coverage
|
||||
When I run plan execute without a plan id with 1 complete plans
|
||||
Then the plan lifecycle command should succeed
|
||||
And the execute command should run the single ready plan
|
||||
And the plan lifecycle output should contain "Execute phase"
|
||||
And the plan lifecycle output should contain "Plan Executed"
|
||||
|
||||
Scenario: Plan execute handles invalid phase transition
|
||||
When I run plan execute for plan id "01ARZ3NDEKTSV4RRFFQ69G5FAV" causing "invalid transition"
|
||||
|
||||
@@ -650,14 +650,12 @@ def step_invoke_execute_plan_queued(context):
|
||||
executed_plan = _make_mock_lifecycle_plan(phase="execute", state="queued")
|
||||
completed_plan = _make_mock_lifecycle_plan(phase="execute", state="complete")
|
||||
|
||||
# get_plan call sequence:
|
||||
# 1. read-only check (pre_plan)
|
||||
# 2. strategize state inspection (current_plan)
|
||||
# 3. re-fetch after inline strategize
|
||||
# 4. inline execute state inspection
|
||||
# 5. re-fetch after inline execute
|
||||
# get_plan call sequence (consolidated read-only + phase check):
|
||||
# 1. current_plan (null + read-only + strategize state)
|
||||
# 2. re-fetch after inline strategize
|
||||
# 3. inline execute state inspection
|
||||
# 4. re-fetch after inline execute
|
||||
mock_service.get_plan.side_effect = [
|
||||
queued_plan,
|
||||
queued_plan,
|
||||
executed_plan,
|
||||
executed_plan,
|
||||
@@ -702,14 +700,12 @@ def step_invoke_execute_plan_auto_progressed(context):
|
||||
executed_plan = _make_mock_lifecycle_plan(phase="execute", state="queued")
|
||||
completed_plan = _make_mock_lifecycle_plan(phase="execute", state="complete")
|
||||
|
||||
# get_plan call sequence:
|
||||
# 1. read-only check (pre_plan)
|
||||
# 2. strategize state inspection (current_plan)
|
||||
# 3. re-fetch after inline strategize → execute/queued
|
||||
# 4. inline execute state inspection → execute/queued (triggers run_execute)
|
||||
# 5. re-fetch after inline execute → execute/complete
|
||||
# get_plan call sequence (consolidated read-only + phase check):
|
||||
# 1. current_plan (null + read-only + strategize state)
|
||||
# 2. re-fetch after inline strategize → execute/queued
|
||||
# 3. inline execute state inspection → execute/queued (triggers run_execute)
|
||||
# 4. re-fetch after inline execute → execute/complete
|
||||
mock_service.get_plan.side_effect = [
|
||||
queued_plan,
|
||||
queued_plan,
|
||||
executed_plan,
|
||||
executed_plan,
|
||||
|
||||
@@ -141,13 +141,11 @@ def step_plan_in_strategize_queued(context: Context) -> None:
|
||||
)
|
||||
|
||||
# get_plan call sequence in the CLI execute_plan handler:
|
||||
# 1. pre_plan (read-only check)
|
||||
# 2. current_plan (phase detection)
|
||||
# 3. re-fetch after run_strategize (auto_progress moved to Execute)
|
||||
# 4. re-fetch for inline execute check
|
||||
# 5. re-fetch after run_execute
|
||||
# 1. current_plan (phase detection + read-only check — merged)
|
||||
# 2. re-fetch after run_strategize (auto_progress moved to Execute)
|
||||
# 3. re-fetch for inline execute check
|
||||
# 4. re-fetch after run_execute
|
||||
context.mock_service_967.get_plan.side_effect = [
|
||||
queued_plan,
|
||||
queued_plan,
|
||||
execute_queued_plan,
|
||||
execute_queued_plan,
|
||||
@@ -340,13 +338,11 @@ def step_single_queued_plan_auto_discovery(context: Context) -> None:
|
||||
]
|
||||
|
||||
# get_plan call sequence after auto-discovery selects the plan:
|
||||
# 1. pre_plan (read-only check)
|
||||
# 2. current_plan (phase detection)
|
||||
# 3. re-fetch after run_strategize
|
||||
# 4. re-fetch for inline execute check
|
||||
# 5. re-fetch after run_execute
|
||||
# 1. current_plan (phase detection + read-only check — merged)
|
||||
# 2. re-fetch after run_strategize
|
||||
# 3. re-fetch for inline execute check
|
||||
# 4. re-fetch after run_execute
|
||||
context.mock_service_967.get_plan.side_effect = [
|
||||
queued_plan,
|
||||
queued_plan,
|
||||
execute_queued_plan,
|
||||
execute_queued_plan,
|
||||
|
||||
@@ -10,7 +10,7 @@ plan lifecycle.
|
||||
| ``agents plan use`` | Create plan from action + project(s) |
|
||||
| ``agents plan lifecycle-list``| List plans with optional filters |
|
||||
| ``agents plan status`` | Show plan status / details |
|
||||
| ``agents plan execute`` | Transition to Execute phase |
|
||||
| ``agents plan execute`` | Run phase-aware plan execution |
|
||||
| ``agents plan lifecycle-apply``| Transition to Apply phase |
|
||||
| ``agents plan cancel`` | Cancel a non-terminal plan |
|
||||
| ``agents plan diff`` | Show ChangeSet as unified diff |
|
||||
@@ -1824,20 +1824,19 @@ def execute_plan(
|
||||
pre.execution_environment = execution_environment.lower()
|
||||
service._commit_plan(pre)
|
||||
|
||||
# Fail-fast: read-only plans must not enter Execute phase
|
||||
pre_plan = service.get_plan(plan_id)
|
||||
if pre_plan is not None and pre_plan.read_only is True:
|
||||
console.print(
|
||||
f"[red]Cannot execute plan '{plan_id}': plan is read-only.[/red]"
|
||||
)
|
||||
raise typer.Abort()
|
||||
|
||||
# Determine current phase and run the appropriate processing
|
||||
current_plan = service.get_plan(plan_id)
|
||||
if current_plan is None:
|
||||
console.print(f"[red]Plan '{plan_id}' not found.[/red]")
|
||||
raise typer.Abort()
|
||||
|
||||
# Fail-fast: read-only plans must not enter Execute phase
|
||||
if current_plan.read_only is True:
|
||||
console.print(
|
||||
f"[red]Cannot execute plan '{plan_id}': plan is read-only.[/red]"
|
||||
)
|
||||
raise typer.Abort()
|
||||
|
||||
# If the plan is still in Strategize/queued, run the strategize
|
||||
# phase inline before transitioning to Execute. This implements
|
||||
# the auto_strategize behaviour described in the specification:
|
||||
@@ -1888,11 +1887,21 @@ def execute_plan(
|
||||
data = _plan_spec_dict(plan)
|
||||
console.print(format_output(data, fmt))
|
||||
else:
|
||||
_print_lifecycle_plan(plan, title="Plan Executing")
|
||||
console.print(
|
||||
"\n[dim]Plan is now in Execute phase. "
|
||||
"Run 'agents plan apply <id>' when ready.[/dim]"
|
||||
)
|
||||
_print_lifecycle_plan(plan, title="Plan Executed")
|
||||
phase_label = f"{plan.phase.value}/{plan.state.value}"
|
||||
if plan.phase == PlanPhase.EXECUTE and plan.state in (
|
||||
ProcessingState.COMPLETE,
|
||||
ProcessingState.APPLIED,
|
||||
):
|
||||
console.print(
|
||||
f"\n[dim]Plan execution completed ({phase_label}). "
|
||||
"Run 'agents plan lifecycle-apply <id>' when ready.[/dim]"
|
||||
)
|
||||
else:
|
||||
console.print(
|
||||
f"\n[dim]Plan is now in {phase_label} state. "
|
||||
"Run 'agents plan execute <id>' to continue.[/dim]"
|
||||
)
|
||||
|
||||
except PreflightRejection as e:
|
||||
console.print(f"[red]Pre-flight check failed:[/red] {e}")
|
||||
|
||||
Reference in New Issue
Block a user