cleveragents/cleveragents-core

Fork 3

fix(cli): add missing `--execution-env-priority` flag to `plan use` #886

New issue

Closed

opened 2026-03-13 23:26:37 +00:00 by freemo · 6 comments

freemo commented

2026-03-13 23:26:37 +00:00

Owner

Metadata

Commit Message: fix(cli): add --execution-env-priority flag to plan use
Branch: feature/m3-plan-use-env-priority

Background and Context

The specification defines the agents plan use command with an --execution-env-priority (fallback|override) flag (CLI Synopsis line 331). This flag controls whether the specified execution environment takes precedence over auto-detected devcontainers (override) or defers to them (fallback). See ADR-043 (DevContainer Integration) for the full precedence semantics.

The current implementation in src/cleveragents/cli/commands/plan.py (lines 1279-1365) includes --execution-environment but does not include --execution-env-priority. The ExecutionEnvironment domain model in domain/models/core/plan.py does include a priority concept, but it is not exposed via the CLI.

Spec reference: --execution-env-priority (fallback|override) on agents plan use

Expected Behavior

agents plan use --execution-environment <RESOURCE> --execution-env-priority override <ACTION> <PROJECT> should set the execution environment priority on the newly created plan.

Acceptance Criteria

--execution-env-priority flag is added to plan use command
Flag accepts exactly two values: fallback and override
Flag value is persisted on the Plan model when creating the plan
Flag is optional; when omitted, defaults to fallback
Flag only takes effect when --execution-environment is also provided
Error message when --execution-env-priority is used without --execution-environment
Output includes the priority setting when displaying plan details

Subtasks

Add --execution-env-priority Typer option to plan use command in plan.py
Pass priority value through to plan_lifecycle_service.py plan creation
Ensure Plan model stores the priority setting correctly
Add validation: error if priority is set without execution environment
Tests (Behave): Scenarios for flag presence, default, and validation
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): add --execution-env-priority flag to plan use` - **Branch**: `feature/m3-plan-use-env-priority` ## Background and Context The specification defines the `agents plan use` command with an `--execution-env-priority (fallback|override)` flag (CLI Synopsis line 331). This flag controls whether the specified execution environment takes precedence over auto-detected devcontainers (`override`) or defers to them (`fallback`). See ADR-043 (DevContainer Integration) for the full precedence semantics. The current implementation in `src/cleveragents/cli/commands/plan.py` (lines 1279-1365) includes `--execution-environment` but does **not** include `--execution-env-priority`. The `ExecutionEnvironment` domain model in `domain/models/core/plan.py` does include a `priority` concept, but it is not exposed via the CLI. **Spec reference**: `--execution-env-priority (fallback|override)` on `agents plan use` ## Expected Behavior `agents plan use --execution-environment <RESOURCE> --execution-env-priority override <ACTION> <PROJECT>` should set the execution environment priority on the newly created plan. ## Acceptance Criteria - [x] `--execution-env-priority` flag is added to `plan use` command - [x] Flag accepts exactly two values: `fallback` and `override` - [x] Flag value is persisted on the Plan model when creating the plan - [x] Flag is optional; when omitted, defaults to `fallback` - [x] Flag only takes effect when `--execution-environment` is also provided - [x] Error message when `--execution-env-priority` is used without `--execution-environment` - [x] Output includes the priority setting when displaying plan details ## Subtasks - [x] Add `--execution-env-priority` Typer option to `plan use` command in `plan.py` - [x] Pass priority value through to `plan_lifecycle_service.py` plan creation - [x] Ensure `Plan` model stores the priority setting correctly - [x] Add validation: error if priority is set without execution environment - [x] Tests (Behave): Scenarios for flag presence, default, and validation - [x] Verify coverage >= 97% via `nox -s coverage_report` - [x] 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 the

labels

2026-03-13 23:30:21 +00:00

freemo added this to the v3.3.0 milestone

2026-03-13 23:30:30 +00:00

freemo commented

2026-03-13 23:30:52 +00:00

Author

Owner

Dependencies / Related Issues:

Related: #879 (tool-level execution environment preferences — same env resolution pipeline)

**Dependencies / Related Issues:** - Related: #879 (tool-level execution environment preferences — same env resolution pipeline)

hurui200320 was assigned by freemo

2026-03-14 04:26:55 +00:00

hurui200320 added

and removed

labels

2026-03-16 05:50:06 +00:00

hurui200320 referenced this issue from a commit

2026-03-16 06:57:23 +00:00

fix(cli): add --execution-env-priority flag to plan use

hurui200320 commented

2026-03-16 08:11:30 +00:00

Member

Implementation Notes — @hurui200320

Design Decisions

ExecutionEnvPriority as StrEnum: Added a new ExecutionEnvPriority StrEnum to cleveragents.domain.models.core.plan (adjacent to the existing ExecutionEnvironment enum) with values FALLBACK = "fallback" and OVERRIDE = "override". This mirrors the pattern used by ExecutionEnvironment and PlanPhase.
Plan model field: Added execution_env_priority: str | None to the Plan model, stored as a nullable string (consistent with execution_environment's typing pattern). Defaults to None; when --execution-environment is set without explicit priority, the CLI defaults to "fallback" per spec.
Validation placement: Validation of the --execution-env-priority flag is in the CLI layer (plan.py use_action command) rather than the domain model. This follows the existing pattern where execution_environment is validated in the CLI before being set on the plan object.
Post-creation setting pattern: Like execution_environment, the priority is set on the plan object after service.use_action() returns. This maintains the existing pattern where execution environment properties are not passed through the service's use_action() method signature.

Key Code Locations (commit `022e43f9`)

cleveragents.domain.models.core.plan.ExecutionEnvPriority — new enum
cleveragents.domain.models.core.plan.Plan.execution_env_priority — new field
cleveragents.cli.commands.plan.use_action — new --execution-env-priority parameter and validation logic
cleveragents.cli.commands.plan._print_lifecycle_plan — updated output to include priority
cleveragents.cli.commands.plan._plan_spec_dict — updated JSON output

Test Coverage

Behave: 7 scenarios in features/plan_use_env_priority.feature covering: override/fallback acceptance, missing environment validation, default fallback behavior, invalid value rejection, and output display in both rich and JSON formats.
Robot Framework: 5 integration tests in robot/plan_use_env_priority.robot.

Quality Gate Results

Session	Result
lint	PASS
typecheck	PASS (0 errors)
unit_tests	PASS (10,822 scenarios)
coverage_report	97% (threshold: 97%)
integration_tests	2 pre-existing flaky failures (master has 19) — unrelated to this change

## Implementation Notes — @hurui200320 ### Design Decisions 1. **`ExecutionEnvPriority` as StrEnum**: Added a new `ExecutionEnvPriority` StrEnum to `cleveragents.domain.models.core.plan` (adjacent to the existing `ExecutionEnvironment` enum) with values `FALLBACK = "fallback"` and `OVERRIDE = "override"`. This mirrors the pattern used by `ExecutionEnvironment` and `PlanPhase`. 2. **Plan model field**: Added `execution_env_priority: str | None` to the `Plan` model, stored as a nullable string (consistent with `execution_environment`'s typing pattern). Defaults to `None`; when `--execution-environment` is set without explicit priority, the CLI defaults to `"fallback"` per spec. 3. **Validation placement**: Validation of the `--execution-env-priority` flag is in the CLI layer (`plan.py` `use_action` command) rather than the domain model. This follows the existing pattern where `execution_environment` is validated in the CLI before being set on the plan object. 4. **Post-creation setting pattern**: Like `execution_environment`, the priority is set on the plan object after `service.use_action()` returns. This maintains the existing pattern where execution environment properties are not passed through the service's `use_action()` method signature. ### Key Code Locations (commit `022e43f9`) - `cleveragents.domain.models.core.plan.ExecutionEnvPriority` — new enum - `cleveragents.domain.models.core.plan.Plan.execution_env_priority` — new field - `cleveragents.cli.commands.plan.use_action` — new `--execution-env-priority` parameter and validation logic - `cleveragents.cli.commands.plan._print_lifecycle_plan` — updated output to include priority - `cleveragents.cli.commands.plan._plan_spec_dict` — updated JSON output ### Test Coverage - **Behave**: 7 scenarios in `features/plan_use_env_priority.feature` covering: override/fallback acceptance, missing environment validation, default fallback behavior, invalid value rejection, and output display in both rich and JSON formats. - **Robot Framework**: 5 integration tests in `robot/plan_use_env_priority.robot`. ### Quality Gate Results | Session | Result | |---------|--------| | lint | PASS | | typecheck | PASS (0 errors) | | unit_tests | PASS (10,822 scenarios) | | coverage_report | 97% (threshold: 97%) | | integration_tests | 2 pre-existing flaky failures (master has 19) — unrelated to this change |

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

2026-03-16 08:11:59 +00:00

fix(cli): add --execution-env-priority flag to plan use #972

hurui200320 added

and removed

labels

2026-03-16 08:12:27 +00:00

hurui200320 referenced this issue

2026-03-16 08:38:02 +00:00

fix(cli): add --execution-env-priority flag to plan use #972

hurui200320 referenced this issue from a commit

2026-03-16 09:13:41 +00:00

fix(cli): add --execution-env-priority flag to plan use

hurui200320 commented

2026-03-16 09:14:52 +00:00

Member

Review Response — @hurui200320 (commit `8df51f61`)

Self-review identified 11 items (3 blocking, 3 major, 5 minor). Addressed all actionable items in an amended commit. Summary:

Fixed (8 items)

CHANGELOG.md — Added entry under ## Unreleased
Model-level validation — Added @model_validator(mode="after") on Plan enforcing execution_env_priority requires execution_environment
as_cli_dict() omission — Added both fields to as_cli_dict() with .value for stable output
Tests don't verify use_action() call — Added assert_called_once() step in Scenario 1
Field type str | None — Changed to ExecutionEnvPriority | None for Pydantic enum validation (consistent with phase: PlanPhase pattern)
No case-insensitive test — Added Scenario 8 testing uppercase "OVERRIDE" input
Fragile Scenario 6 — Strengthened to assert both label and value present
Aggressive sys.path pattern — Simplified Robot helper to standard sys.path.insert(0, _SRC) approach

Deferred (3 items)

DB persistence gap — Pre-existing architectural gap affecting all CLI overrides. Separate issue needed.
Robot mocks in integration tests — Pre-existing codebase-wide pattern (20+ helpers). Separate comprehensive effort needed.
Naming inconsistency — Flag name --execution-env-priority is spec-driven (CLI Synopsis line 331). Cannot change without spec amendment.

Quality Gates (post-fix)

Session	Result
lint	PASS
typecheck	PASS (0 errors)
unit_tests	PASS (10,823 scenarios, 0 failures)
coverage_report	97%

## Review Response — @hurui200320 (commit `8df51f61`) Self-review identified 11 items (3 blocking, 3 major, 5 minor). Addressed all actionable items in an amended commit. Summary: ### Fixed (8 items) 1. **CHANGELOG.md** — Added entry under `## Unreleased` 2. **Model-level validation** — Added `@model_validator(mode="after")` on `Plan` enforcing `execution_env_priority` requires `execution_environment` 3. **`as_cli_dict()` omission** — Added both fields to `as_cli_dict()` with `.value` for stable output 4. **Tests don't verify `use_action()` call** — Added `assert_called_once()` step in Scenario 1 5. **Field type `str | None`** — Changed to `ExecutionEnvPriority | None` for Pydantic enum validation (consistent with `phase: PlanPhase` pattern) 6. **No case-insensitive test** — Added Scenario 8 testing uppercase `"OVERRIDE"` input 7. **Fragile Scenario 6** — Strengthened to assert both label and value present 8. **Aggressive `sys.path` pattern** — Simplified Robot helper to standard `sys.path.insert(0, _SRC)` approach ### Deferred (3 items) - **DB persistence gap** — Pre-existing architectural gap affecting all CLI overrides. Separate issue needed. - **Robot mocks in integration tests** — Pre-existing codebase-wide pattern (20+ helpers). Separate comprehensive effort needed. - **Naming inconsistency** — Flag name `--execution-env-priority` is spec-driven (CLI Synopsis line 331). Cannot change without spec amendment. ### Quality Gates (post-fix) | Session | Result | |---------|--------| | lint | PASS | | typecheck | PASS (0 errors) | | unit_tests | PASS (10,823 scenarios, 0 failures) | | coverage_report | 97% |

hurui200320 referenced this issue from a commit

2026-03-16 10:20:18 +00:00

fix(cli): add --execution-env-priority flag to plan use

hurui200320 commented

2026-03-16 10:21:14 +00:00

Member

Implementation Notes — Persistence Fix (commit `ad3e4708`)

Context

The self-review on PR !972 identified that execution_env_priority (and execution_environment) were not persisted to the database. The initial response deferred this as a pre-existing architectural gap, but acceptance criterion "Flag value is persisted on the Plan model when creating the plan" explicitly requires persistence. This has now been resolved.

Design Decision: Re-persist after mutation

The CLI use_action handler follows a two-phase pattern:

Phase 1: service.use_action() creates and persists the plan to the database with default field values.
Phase 2: The CLI applies CLI-provided overrides (execution_environment, execution_env_priority, actors, automation profile) to the in-memory plan object.

The persistence gap was that Phase 2 mutations were never written back to the database. The fix adds service.save_plan(plan) after all overrides are applied, which opens a new UoW transaction and calls repository.update().

Why not pass values through use_action()? This would require changing the use_action() signature to accept optional overrides, which affects all callers. The re-persist approach is less invasive and works for all override types, not just env priority. It also avoids coupling service API design to CLI parameter evolution.

Implementation Details

Component	Change	Location
`LifecyclePlanModel`	Added `execution_environment` and `execution_env_priority` columns	`infrastructure/database/models.py`, columns section
`LifecyclePlanModel.to_domain()`	Reconstructs `ExecutionEnvPriority` enum from stored string	`infrastructure/database/models.py`, `to_domain()` method
`LifecyclePlanModel.from_domain()`	Serializes enum `.value` with null guard	`infrastructure/database/models.py`, `from_domain()` method
`LifecyclePlanRepository.update()`	Writes both fields using `getattr` + `.value` pattern	`infrastructure/database/repositories.py`, `update()` method
`PlanLifecycleService.save_plan()`	Public re-persist method delegating to `_commit_plan()`	`application/services/plan_lifecycle_service.py`
CLI `use_action`	Calls `service.save_plan(plan)` after overrides	`cli/commands/plan.py`, `use_action()` function
Alembic migration	`m4_003_plan_env_columns` adding both columns	`alembic/versions/m4_003_plan_env_columns.py`

Test Verification

Behave Scenario 1 and 4 now assert save_plan.assert_called_once().
All 10,823 unit test scenarios pass, all 1,516 integration tests pass.
Coverage remains at 97%.

## Implementation Notes — Persistence Fix (commit `ad3e4708`) ### Context The self-review on PR !972 identified that `execution_env_priority` (and `execution_environment`) were not persisted to the database. The initial response deferred this as a pre-existing architectural gap, but acceptance criterion "Flag value is persisted on the Plan model when creating the plan" explicitly requires persistence. This has now been resolved. ### Design Decision: Re-persist after mutation The CLI `use_action` handler follows a two-phase pattern: 1. **Phase 1**: `service.use_action()` creates and persists the plan to the database with default field values. 2. **Phase 2**: The CLI applies CLI-provided overrides (execution_environment, execution_env_priority, actors, automation profile) to the in-memory plan object. The persistence gap was that Phase 2 mutations were never written back to the database. The fix adds `service.save_plan(plan)` after all overrides are applied, which opens a new UoW transaction and calls `repository.update()`. **Why not pass values through `use_action()`?** This would require changing the `use_action()` signature to accept optional overrides, which affects all callers. The re-persist approach is less invasive and works for all override types, not just env priority. It also avoids coupling service API design to CLI parameter evolution. ### Implementation Details | Component | Change | Location | |-----------|--------|----------| | `LifecyclePlanModel` | Added `execution_environment` and `execution_env_priority` columns | `infrastructure/database/models.py`, columns section | | `LifecyclePlanModel.to_domain()` | Reconstructs `ExecutionEnvPriority` enum from stored string | `infrastructure/database/models.py`, `to_domain()` method | | `LifecyclePlanModel.from_domain()` | Serializes enum `.value` with null guard | `infrastructure/database/models.py`, `from_domain()` method | | `LifecyclePlanRepository.update()` | Writes both fields using `getattr` + `.value` pattern | `infrastructure/database/repositories.py`, `update()` method | | `PlanLifecycleService.save_plan()` | Public re-persist method delegating to `_commit_plan()` | `application/services/plan_lifecycle_service.py` | | CLI `use_action` | Calls `service.save_plan(plan)` after overrides | `cli/commands/plan.py`, `use_action()` function | | Alembic migration | `m4_003_plan_env_columns` adding both columns | `alembic/versions/m4_003_plan_env_columns.py` | ### Test Verification - Behave Scenario 1 and 4 now assert `save_plan.assert_called_once()`. - All 10,823 unit test scenarios pass, all 1,516 integration tests pass. - Coverage remains at 97%.

hurui200320 referenced this issue

2026-03-16 12:16:35 +00:00

test(e2e): E2E acceptance criteria for M5 (v3.4.0) — ACMS v1 and context scaling #811

hurui200320 referenced this issue

2026-03-16 12:16:47 +00:00

test(e2e): E2E acceptance criteria for M5 (v3.4.0) — ACMS v1 and context scaling #811

hurui200320 referenced this issue

2026-03-16 12:17:04 +00:00

test(e2e): E2E acceptance criteria for M5 (v3.4.0) — ACMS v1 and context scaling #745

hurui200320 referenced this issue from a commit

2026-03-17 07:28:03 +00:00

fix(cli): add --execution-env-priority flag to plan use

hurui200320 commented

2026-03-17 07:41:39 +00:00

Member

Self-QA Implementation Notes (Cycles 1–2)

Cycle 1

Review findings (17 items):

Major (6): validate_assignment=True possibly missing on Plan class; String(20) column too narrow for future resource names; no test for @model_validator invariant; no test for DB round-trip serialization; no test for Plan.as_cli_dict() with new fields; save_plan assertion verifies call count but not arguments.
Minor (6): save_plan() called unconditionally; partial failure atomicity concern; duplicate deferred import; pre-migration data default omitted; Alembic naming inconsistency; no ExecutionEnvPriority enum tests.
Nits (3): Defensive getattr() on well-defined fields (consistent with codebase convention); _plan_spec_dict docstring incomplete; duplicate test constant.

Fixes applied:

Confirmed validate_assignment=True already present in Plan.model_config — no code change needed.
Changed String(20) → String(255) for execution_environment column in both LifecyclePlanModel and Alembic migration m4_003_plan_env_columns.py.
Added 2 Behave scenarios testing @model_validator (priority without environment raises ValueError; both fields set succeeds).
Added 2 Behave scenarios testing LifecyclePlanModel.from_domain() → to_domain() round-trip for both set and None values.
Added 3 Behave scenarios for Plan.as_cli_dict() (both fields set, both None, pre-migration fallback default).
Strengthened save_plan assertion to verify call_args[0][0] is context.mock_plan.
Added has_overrides guard — save_plan() now only called when CLI overrides are actually applied.
Hoisted duplicate ExecutionEnvPriority import above conditional branches.
Added pre-migration fallback default ("fallback") in Plan.as_cli_dict(), _plan_spec_dict(), and _print_lifecycle_plan().
Added 3 Behave scenarios verifying ExecutionEnvPriority enum values and StrEnum subclass.
Updated _plan_spec_dict docstring to include new keys.

Deferred:

Partial failure atomicity (save_plan failure leaves plan without CLI overrides) — requires service-layer restructuring beyond scope of #886.
Alembic migration naming (m4_003 depends on m6_004) — renaming risks breaking the migration chain; cosmetic only.

Cycle 2

Review findings: Verdict Approve. All 11 Cycle 1 fixes verified correct. Remaining findings are minor test quality improvements and code style refinements (triplicated fallback logic could be DRY'd, overly broad exception handling in validator test step, deferred import placement, missing save_plan assertion on Scenario 2, help text could be more descriptive). None affect functional correctness.

Quality Gates (Final)

Session	Result
`nox -e lint`	✅ PASS
`nox -e typecheck`	✅ PASS (0 errors)
`nox -e unit_tests`	✅ PASS (10,833 scenarios)
`nox -e integration_tests`	✅ PASS (1,516 tests)
`nox -e coverage_report`	✅ PASS (97%)

Remaining Issues (Minor, Non-blocking)

Triplicated fallback-default logic could be consolidated into a Plan.effective_env_priority property.
Validator test step uses overly broad except (ValueError, Exception) — could be narrowed.
ExecutionEnvPriority import in use_action() is unconditionally hoisted — could be guarded for consistency.
Scenario 2 (fallback path) missing save_plan assertion.
Repository update() path not independently tested for new fields.
Pre-migration fallback not tested in _plan_spec_dict() and _print_lifecycle_plan() output paths.

## Self-QA Implementation Notes (Cycles 1–2) ### Cycle 1 **Review findings (17 items):** - **Major (6):** `validate_assignment=True` possibly missing on `Plan` class; `String(20)` column too narrow for future resource names; no test for `@model_validator` invariant; no test for DB round-trip serialization; no test for `Plan.as_cli_dict()` with new fields; `save_plan` assertion verifies call count but not arguments. - **Minor (6):** `save_plan()` called unconditionally; partial failure atomicity concern; duplicate deferred import; pre-migration data default omitted; Alembic naming inconsistency; no `ExecutionEnvPriority` enum tests. - **Nits (3):** Defensive `getattr()` on well-defined fields (consistent with codebase convention); `_plan_spec_dict` docstring incomplete; duplicate test constant. **Fixes applied:** 1. **Confirmed** `validate_assignment=True` already present in `Plan.model_config` — no code change needed. 2. **Changed** `String(20)` → `String(255)` for `execution_environment` column in both `LifecyclePlanModel` and Alembic migration `m4_003_plan_env_columns.py`. 3. **Added** 2 Behave scenarios testing `@model_validator` (priority without environment raises `ValueError`; both fields set succeeds). 4. **Added** 2 Behave scenarios testing `LifecyclePlanModel.from_domain()` → `to_domain()` round-trip for both set and `None` values. 5. **Added** 3 Behave scenarios for `Plan.as_cli_dict()` (both fields set, both `None`, pre-migration fallback default). 6. **Strengthened** `save_plan` assertion to verify `call_args[0][0] is context.mock_plan`. 7. **Added** `has_overrides` guard — `save_plan()` now only called when CLI overrides are actually applied. 8. **Hoisted** duplicate `ExecutionEnvPriority` import above conditional branches. 9. **Added** pre-migration fallback default (`"fallback"`) in `Plan.as_cli_dict()`, `_plan_spec_dict()`, and `_print_lifecycle_plan()`. 10. **Added** 3 Behave scenarios verifying `ExecutionEnvPriority` enum values and `StrEnum` subclass. 11. **Updated** `_plan_spec_dict` docstring to include new keys. **Deferred:** - Partial failure atomicity (`save_plan` failure leaves plan without CLI overrides) — requires service-layer restructuring beyond scope of #886. - Alembic migration naming (`m4_003` depends on `m6_004`) — renaming risks breaking the migration chain; cosmetic only. ### Cycle 2 **Review findings:** Verdict **Approve**. All 11 Cycle 1 fixes verified correct. Remaining findings are minor test quality improvements and code style refinements (triplicated fallback logic could be DRY'd, overly broad exception handling in validator test step, deferred import placement, missing `save_plan` assertion on Scenario 2, help text could be more descriptive). None affect functional correctness. ### Quality Gates (Final) | Session | Result | |---------|--------| | `nox -e lint` | ✅ PASS | | `nox -e typecheck` | ✅ PASS (0 errors) | | `nox -e unit_tests` | ✅ PASS (10,833 scenarios) | | `nox -e integration_tests` | ✅ PASS (1,516 tests) | | `nox -e coverage_report` | ✅ PASS (97%) | ### Remaining Issues (Minor, Non-blocking) 1. Triplicated fallback-default logic could be consolidated into a `Plan.effective_env_priority` property. 2. Validator test step uses overly broad `except (ValueError, Exception)` — could be narrowed. 3. `ExecutionEnvPriority` import in `use_action()` is unconditionally hoisted — could be guarded for consistency. 4. Scenario 2 (fallback path) missing `save_plan` assertion. 5. Repository `update()` path not independently tested for new fields. 6. Pre-migration fallback not tested in `_plan_spec_dict()` and `_print_lifecycle_plan()` output paths.

hurui200320 referenced this issue

2026-03-17 07:59:29 +00:00

fix(cli): add --execution-env-priority flag to plan use #972

hurui200320 referenced this issue from a commit

2026-03-17 08:19:14 +00:00

fix(cli): add --execution-env-priority flag to plan use

freemo referenced this issue

2026-03-17 18:22:36 +00:00

test(e2e): E2E acceptance criteria for M5 (v3.4.0) — ACMS v1 and context scaling #811

hurui200320 referenced this issue from a commit

2026-03-18 07:57:40 +00:00

fix(cli): add --execution-env-priority flag to plan use

hurui200320 commented

2026-03-18 07:58:55 +00:00

Member

Implementation Notes — Review Fix Round (Brent Edwards Review #2384)

Addressed two P2 should-fix items from Brent's review, rebased onto latest master, and force-pushed.

Changes

P2 #1 — Direct attribute access: Replaced defensive getattr(plan, "execution_environment", None) with plan.execution_environment in LifecyclePlanModel.from_domain() and LifecyclePlanRepository.update(). Plan fields always exist on the Pydantic BaseModel. Updated 4 pre-existing SimpleNamespace-based test fixtures (database_models_new_coverage_steps, database_models_lifecycle_coverage_steps, database_models_coverage_r2_steps, repositories_error_handling_coverage_steps) to include execution_environment=None and execution_env_priority=None attributes.

P2 #2 — Hoisted import: Moved ExecutionEnvPriority import from conditional blocks to function-entry deferred imports in _plan_spec_dict() and _print_lifecycle_plan().

Rebase: Branch rebased onto latest master with CHANGELOG.md conflict resolved. Alembic migration m4_003 now descends from m6_005_profile_guards_json to fix post-rebase multiple-heads issue.

Quality Gates

All passing: lint ✅ | typecheck ✅ | unit_tests (11,125 scenarios) ✅ | integration_tests (1,562 tests) ✅ | coverage 97% ✅

## Implementation Notes — Review Fix Round (Brent Edwards Review #2384) Addressed two P2 should-fix items from Brent's review, rebased onto latest master, and force-pushed. ### Changes **P2 #1 — Direct attribute access**: Replaced defensive `getattr(plan, "execution_environment", None)` with `plan.execution_environment` in `LifecyclePlanModel.from_domain()` and `LifecyclePlanRepository.update()`. Plan fields always exist on the Pydantic BaseModel. Updated 4 pre-existing `SimpleNamespace`-based test fixtures (`database_models_new_coverage_steps`, `database_models_lifecycle_coverage_steps`, `database_models_coverage_r2_steps`, `repositories_error_handling_coverage_steps`) to include `execution_environment=None` and `execution_env_priority=None` attributes. **P2 #2 — Hoisted import**: Moved `ExecutionEnvPriority` import from conditional blocks to function-entry deferred imports in `_plan_spec_dict()` and `_print_lifecycle_plan()`. **Rebase**: Branch rebased onto latest master with `CHANGELOG.md` conflict resolved. Alembic migration `m4_003` now descends from `m6_005_profile_guards_json` to fix post-rebase multiple-heads issue. ### Quality Gates All passing: lint ✅ | typecheck ✅ | unit_tests (11,125 scenarios) ✅ | integration_tests (1,562 tests) ✅ | coverage 97% ✅

hurui200320 referenced this issue from a commit

2026-03-18 08:03:34 +00:00

fix(cli): add --execution-env-priority flag to plan use

hurui200320 closed this issue

2026-03-18 08:13:56 +00:00

hurui200320 referenced this issue from a commit

2026-03-18 08:13:57 +00:00

fix(cli): add --execution-env-priority flag to plan use (#972)

hurui200320 added

State

Completed

and removed