Files
temp/robot/helper_plan_use_env_priority.py
hurui200320 48ecf4c00c fix(cli): add --execution-env-priority flag to plan use (#972)
## 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: cleveragents/cleveragents-core#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>
2026-03-18 08:13:56 +00:00

279 lines
8.8 KiB
Python

"""Helper script for plan_use_env_priority.robot smoke tests.
Each subcommand is a self-contained check that prints a sentinel on success.
"""
from __future__ import annotations
import sys
from datetime import datetime
from pathlib import Path
from unittest.mock import MagicMock, patch
# Ensure the local source tree takes priority over any installed copy.
_SRC = str(Path(__file__).resolve().parents[1] / "src")
if _SRC not in sys.path:
sys.path.insert(0, _SRC)
from typer.testing import CliRunner # noqa: E402
from cleveragents.cli.commands.plan import app as plan_app # noqa: E402
from cleveragents.domain.models.core.action import Action, ActionState # noqa: E402
from cleveragents.domain.models.core.plan import ( # noqa: E402
NamespacedName,
Plan,
PlanIdentity,
PlanPhase,
PlanTimestamps,
ProcessingState,
ProjectLink,
)
runner = CliRunner()
_PLAN_ULID = "01KHDE6WWS2171PWW3GJEBXZ8S"
def _mock_action(name: str = "local/smoke-action") -> Action:
return Action(
namespaced_name=NamespacedName.parse(name),
description="Smoke test action",
long_description=None,
definition_of_done="All smoke tests pass",
strategy_actor="openai/gpt-4",
execution_actor="openai/gpt-4",
reusable=True,
read_only=False,
state=ActionState.AVAILABLE,
created_by=None,
created_at=datetime.now(),
updated_at=datetime.now(),
)
def _mock_plan(
project_links: list[ProjectLink] | None = None,
) -> Plan:
now = datetime.now()
return Plan(
identity=PlanIdentity(plan_id=_PLAN_ULID),
namespaced_name=NamespacedName.parse("local/smoke-plan"),
description="Smoke test plan",
definition_of_done="All smoke tests pass",
action_name="local/smoke-action",
phase=PlanPhase.STRATEGIZE,
processing_state=ProcessingState.QUEUED,
project_links=project_links or [ProjectLink(project_name="proj-a")],
arguments={},
arguments_order=[],
strategy_actor="openai/gpt-4",
execution_actor="openai/gpt-4",
reusable=True,
read_only=False,
created_by=None,
timestamps=PlanTimestamps(created_at=now, updated_at=now),
)
# ---------------------------------------------------------------------------
# Subcommands
# ---------------------------------------------------------------------------
def use_env_priority_override() -> None:
"""Verify plan use accepts --execution-env-priority override."""
mock_service = MagicMock()
mock_service.get_action_by_name.return_value = _mock_action()
mock_service.use_action.return_value = _mock_plan()
with patch(
"cleveragents.cli.commands.plan._get_lifecycle_service",
return_value=mock_service,
):
result = runner.invoke(
plan_app,
[
"use",
"local/smoke-action",
"proj-a",
"--execution-environment",
"container",
"--execution-env-priority",
"override",
],
)
if result.exit_code == 0:
plan = mock_service.use_action.return_value
if plan.execution_env_priority == "override":
print("plan-env-priority-override-ok")
else:
print(
f"FAIL: priority={plan.execution_env_priority!r}",
file=sys.stderr,
)
sys.exit(1)
else:
print(f"FAIL: exit code {result.exit_code}", file=sys.stderr)
print(result.output, file=sys.stderr)
sys.exit(1)
def use_env_priority_fallback() -> None:
"""Verify plan use accepts --execution-env-priority fallback."""
mock_service = MagicMock()
mock_service.get_action_by_name.return_value = _mock_action()
mock_service.use_action.return_value = _mock_plan()
with patch(
"cleveragents.cli.commands.plan._get_lifecycle_service",
return_value=mock_service,
):
result = runner.invoke(
plan_app,
[
"use",
"local/smoke-action",
"proj-a",
"--execution-environment",
"host",
"--execution-env-priority",
"fallback",
],
)
if result.exit_code == 0:
plan = mock_service.use_action.return_value
if plan.execution_env_priority == "fallback":
print("plan-env-priority-fallback-ok")
else:
print(
f"FAIL: priority={plan.execution_env_priority!r}",
file=sys.stderr,
)
sys.exit(1)
else:
print(f"FAIL: exit code {result.exit_code}", file=sys.stderr)
print(result.output, file=sys.stderr)
sys.exit(1)
def use_priority_no_env() -> None:
"""Verify --execution-env-priority without --execution-environment fails."""
mock_service = MagicMock()
mock_service.get_action_by_name.return_value = _mock_action()
mock_service.use_action.return_value = _mock_plan()
with patch(
"cleveragents.cli.commands.plan._get_lifecycle_service",
return_value=mock_service,
):
result = runner.invoke(
plan_app,
[
"use",
"local/smoke-action",
"proj-a",
"--execution-env-priority",
"override",
],
)
if (
result.exit_code != 0
and "--execution-env-priority requires" in result.output
):
print("plan-env-priority-no-env-ok")
else:
print(
f"FAIL: expected failure, got exit code {result.exit_code}",
file=sys.stderr,
)
print(result.output, file=sys.stderr)
sys.exit(1)
def use_env_default_priority() -> None:
"""Verify env without priority defaults to fallback."""
mock_service = MagicMock()
mock_service.get_action_by_name.return_value = _mock_action()
mock_service.use_action.return_value = _mock_plan()
with patch(
"cleveragents.cli.commands.plan._get_lifecycle_service",
return_value=mock_service,
):
result = runner.invoke(
plan_app,
[
"use",
"local/smoke-action",
"proj-a",
"--execution-environment",
"container",
],
)
if result.exit_code == 0:
plan = mock_service.use_action.return_value
if plan.execution_env_priority == "fallback":
print("plan-env-default-priority-ok")
else:
print(
f"FAIL: expected fallback, got {plan.execution_env_priority!r}",
file=sys.stderr,
)
sys.exit(1)
else:
print(f"FAIL: exit code {result.exit_code}", file=sys.stderr)
print(result.output, file=sys.stderr)
sys.exit(1)
def use_invalid_priority() -> None:
"""Verify invalid --execution-env-priority value is rejected."""
mock_service = MagicMock()
mock_service.get_action_by_name.return_value = _mock_action()
mock_service.use_action.return_value = _mock_plan()
with patch(
"cleveragents.cli.commands.plan._get_lifecycle_service",
return_value=mock_service,
):
result = runner.invoke(
plan_app,
[
"use",
"local/smoke-action",
"proj-a",
"--execution-environment",
"container",
"--execution-env-priority",
"invalid-value",
],
)
if result.exit_code != 0 and "Invalid execution env priority" in result.output:
print("plan-env-invalid-priority-ok")
else:
print(
f"FAIL: expected failure, got exit code {result.exit_code}",
file=sys.stderr,
)
print(result.output, file=sys.stderr)
sys.exit(1)
# ---------------------------------------------------------------------------
# Main dispatcher
# ---------------------------------------------------------------------------
_COMMANDS = {
"use-env-priority-override": use_env_priority_override,
"use-env-priority-fallback": use_env_priority_fallback,
"use-priority-no-env": use_priority_no_env,
"use-env-default-priority": use_env_default_priority,
"use-invalid-priority": use_invalid_priority,
}
def main() -> None:
if len(sys.argv) < 2 or sys.argv[1] not in _COMMANDS:
print(f"Usage: {sys.argv[0]} <{'|'.join(_COMMANDS)}>", file=sys.stderr)
sys.exit(2)
_COMMANDS[sys.argv[1]]()
if __name__ == "__main__":
main()