hurui200320
48ecf4c00c
CI / lint (push) Successful in 19s
CI / build (push) Successful in 29s
CI / quality (push) Successful in 29s
CI / security (push) Successful in 43s
CI / typecheck (push) Successful in 54s
CI / benchmark-regression (push) Has been skipped
CI / unit_tests (push) Successful in 2m53s
CI / integration_tests (push) Successful in 3m39s
CI / docker (push) Successful in 56s
CI / e2e_tests (push) Successful in 3m53s
CI / coverage (push) Successful in 6m37s
CI / benchmark-publish (push) Successful in 22m2s
## Summary
Adds the missing `--execution-env-priority` flag to the `agents plan use` command, aligning the CLI with the specification (spec line 12501). The flag accepts `fallback` (default) or `override` and controls execution environment routing precedence per ADR-043:
- **`override`**: The specified execution environment always wins, bypassing devcontainer auto-detection.
- **`fallback`**: The specified environment defers to auto-detected devcontainers or project-level overrides.
### Changes
- **Domain model** (`cleveragents.domain.models.core.plan`):
- Added `ExecutionEnvPriority` StrEnum with `FALLBACK`/`OVERRIDE` values.
- Changed `execution_env_priority` field type to `ExecutionEnvPriority | None` (leverages Pydantic enum validation).
- Added `@model_validator` enforcing that `execution_env_priority` requires `execution_environment` (domain-level fail-fast invariant). Verified `validate_assignment=True` is set on `Plan.model_config`.
- Updated `Plan.as_cli_dict()` to include `execution_environment` and `execution_env_priority`, with fallback default of `"fallback"` for pre-migration data where `execution_env_priority` is `None`.
- **CLI** (`cleveragents.cli.commands.plan`):
- Added `--execution-env-priority` parameter to `use_action`.
- Validation: priority requires `--execution-environment`, enum value validation, case-insensitive input.
- Defaults to `"fallback"` when `--execution-environment` is set without explicit priority.
- Updated `_print_lifecycle_plan` and `_plan_spec_dict` to display the priority, defaulting to `"fallback"` for pre-migration data.
- Updated `_plan_spec_dict` docstring to mention new keys.
- Hoisted `ExecutionEnvPriority` import to function-entry deferred imports in both `_plan_spec_dict` and `_print_lifecycle_plan` (no longer conditional on execution environment being set).
- Guarded `service.save_plan(plan)` with `has_overrides` flag so it is only called when CLI overrides were actually applied.
- **Persistence** (`cleveragents.infrastructure.database`):
- Added `execution_environment` (`String(255)`, nullable) and `execution_env_priority` (`String(20)`, nullable) columns to `LifecyclePlanModel`.
- Updated `from_domain()`/`to_domain()` for round-trip serialization including `ExecutionEnvPriority` enum reconstruction. Uses direct attribute access (`plan.execution_environment`) instead of defensive `getattr` — Plan fields always exist on the Pydantic BaseModel.
- Updated `LifecyclePlanRepository.update()` to persist both fields using direct attribute access.
- Added Alembic migration `m4_003_plan_env_columns` adding both columns to the `v3_plans` table. Uses `String(255)` for `execution_environment` to accommodate namespaced resource names. Descends from `m6_005_profile_guards_json`.
- **Service** (`cleveragents.application.services.plan_lifecycle_service`):
- Added `save_plan()` public convenience method for callers that need to re-persist after post-creation mutations.
- **Tests**:
- 18 Behave scenarios covering:
- CLI acceptance criteria (valid values, defaults, validation errors, output display, case-insensitive input, service invocation with `call_args` verification).
- Domain model validator invariant: construction with priority but no environment raises `ValueError`; construction with both fields succeeds.
- `ExecutionEnvPriority` enum: values verification, `StrEnum` subclass assertion.
- `Plan.as_cli_dict()`: includes both fields when set, omits both when `None`, defaults priority to `"fallback"` for pre-migration data.
- DB round-trip serialization: `from_domain()` → `to_domain()` preserves both fields; preserves `None` values.
- 5 Robot Framework integration tests.
- Updated pre-existing `SimpleNamespace`-based plan test fixtures in `database_models_lifecycle_coverage_steps`, `database_models_new_coverage_steps`, `database_models_coverage_r2_steps`, and `repositories_error_handling_coverage_steps` to include `execution_environment` and `execution_env_priority` attributes.
- Simplified Robot helper `sys.path` pattern to standard approach.
- **Changelog**: Updated per CONTRIBUTING.md requirements.
### Review Fixes (Brent Edwards, Review #2384)
- **P2 #1 — Defensive `getattr`**: Replaced `getattr(plan, "execution_environment", None)` with direct `plan.execution_environment` access in both `LifecyclePlanModel.from_domain()` and `LifecyclePlanRepository.update()`. Plan is a Pydantic BaseModel with `default=None`, so the field always exists. Updated 4 pre-existing `SimpleNamespace`-based test fixtures to include the new attributes.
- **P2 #2 — Late conditional import**: Hoisted `ExecutionEnvPriority` import from inside conditional blocks to function-entry deferred imports in both `_plan_spec_dict` and `_print_lifecycle_plan`.
- **P3 — Migration naming**: Acknowledged as deferred (cosmetic only). Updated `m4_003` to descend from `m6_005_profile_guards_json` (post-rebase chain fix).
- **Rebase**: Branch rebased onto latest `master` with merge conflict in `CHANGELOG.md` resolved.
### Deferred Items
- **Partial failure atomicity** (#8 from review): If `save_plan()` fails after `use_action()` succeeds, the plan exists in the DB without CLI overrides. This requires service-layer restructuring beyond the scope of this ticket.
- **Alembic migration naming** (#11 from review): Migration `m4_003` depends on `m6_005`, creating non-sequential naming. Renaming an existing migration risks breaking the chain for anyone who has already applied it.
### Quality Gates
| Session | Result |
|---------|--------|
| lint | PASS |
| typecheck | PASS (0 errors) |
| unit_tests | PASS (11,125 scenarios, 0 failures) |
| integration_tests | PASS (1,562 tests, 0 failures) |
| coverage_report | 97% (threshold: 97%) |
Closes #886
Reviewed-on: #972
Reviewed-by: Brent Edwards <brent.edwards@cleverthis.com>
Co-authored-by: Rui Hu <rui.hu@cleverthis.com>
Co-committed-by: Rui Hu <rui.hu@cleverthis.com>
42 lines
2.2 KiB
Plaintext
42 lines
2.2 KiB
Plaintext
*** Settings ***
|
|
Documentation Smoke tests for plan use --execution-env-priority flag
|
|
Resource ${CURDIR}/common.resource
|
|
Suite Setup Setup Test Environment
|
|
Suite Teardown Cleanup Test Environment
|
|
|
|
*** Variables ***
|
|
${HELPER} ${CURDIR}/helper_plan_use_env_priority.py
|
|
|
|
*** Test Cases ***
|
|
Plan Use Accepts Execution Env Priority Override
|
|
[Documentation] Verify that ``plan use --execution-env-priority override`` works with --execution-environment
|
|
${result}= Run Process ${PYTHON} ${HELPER} use-env-priority-override cwd=${WORKSPACE}
|
|
Log ${result.stdout}
|
|
Log ${result.stderr}
|
|
Should Be Equal As Integers ${result.rc} 0
|
|
Should Contain ${result.stdout} plan-env-priority-override-ok
|
|
|
|
Plan Use Accepts Execution Env Priority Fallback
|
|
[Documentation] Verify that ``plan use --execution-env-priority fallback`` works with --execution-environment
|
|
${result}= Run Process ${PYTHON} ${HELPER} use-env-priority-fallback cwd=${WORKSPACE}
|
|
Should Be Equal As Integers ${result.rc} 0
|
|
Should Contain ${result.stdout} plan-env-priority-fallback-ok
|
|
|
|
Plan Use Rejects Priority Without Environment
|
|
[Documentation] Verify that ``plan use --execution-env-priority`` without --execution-environment fails
|
|
${result}= Run Process ${PYTHON} ${HELPER} use-priority-no-env cwd=${WORKSPACE}
|
|
Should Be Equal As Integers ${result.rc} 0
|
|
Should Contain ${result.stdout} plan-env-priority-no-env-ok
|
|
|
|
Plan Use Defaults Priority To Fallback
|
|
[Documentation] Verify that --execution-environment without --execution-env-priority defaults to fallback
|
|
${result}= Run Process ${PYTHON} ${HELPER} use-env-default-priority cwd=${WORKSPACE}
|
|
Should Be Equal As Integers ${result.rc} 0
|
|
Should Contain ${result.stdout} plan-env-default-priority-ok
|
|
|
|
Plan Use Rejects Invalid Priority Value
|
|
[Documentation] Verify that invalid --execution-env-priority value is rejected
|
|
${result}= Run Process ${PYTHON} ${HELPER} use-invalid-priority cwd=${WORKSPACE}
|
|
Should Be Equal As Integers ${result.rc} 0
|
|
Should Contain ${result.stdout} plan-env-invalid-priority-ok
|