fix(cli): disallow mixing legacy and v3 plan workflows
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 18s
CI / helm (pull_request) Successful in 23s
CI / lint (pull_request) Failing after 26s
CI / quality (pull_request) Successful in 34s
CI / security (pull_request) Failing after 46s
CI / typecheck (pull_request) Failing after 50s
CI / coverage (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Has been skipped
CI / unit_tests (pull_request) Failing after 1m46s
CI / docker (pull_request) Has been skipped
CI / e2e_tests (pull_request) Failing after 14m9s
CI / integration_tests (pull_request) Failing after 20m57s
CI / status-check (pull_request) Failing after 1s
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 18s
CI / helm (pull_request) Successful in 23s
CI / lint (pull_request) Failing after 26s
CI / quality (pull_request) Successful in 34s
CI / security (pull_request) Failing after 46s
CI / typecheck (pull_request) Failing after 50s
CI / coverage (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Has been skipped
CI / unit_tests (pull_request) Failing after 1m46s
CI / docker (pull_request) Has been skipped
CI / e2e_tests (pull_request) Failing after 14m9s
CI / integration_tests (pull_request) Failing after 20m57s
CI / status-check (pull_request) Failing after 1s
Add ULID format validation to all v3 plan commands to prevent users from
accidentally mixing legacy ('agents tell') and v3 ('agents plan use')
workflows. The two systems use separate storage backends and cannot be
mixed; this change detects the mismatch early and provides actionable
error messages.
Changes:
- Add _validate_plan_ulid() with proper Crockford Base32 regex
(^[0-9A-HJKMNP-TV-Z]{26}$, re.IGNORECASE) that correctly rejects
invalid characters (I, L, O, U), hyphens, and wrong-length strings
- Add _PLAN_ULID_RE compiled regex and _ULID_VALIDATION_ERROR_MSG constant
with actionable guidance explaining the legacy/v3 incompatibility
- Apply ULID validation to all v3 commands: execute_plan,
_lifecycle_apply_with_id, lifecycle_apply_plan, plan_status,
plan_errors, cancel_plan (only on user-provided IDs, not auto-discovered)
- Update _LEGACY_DEPRECATION_MSG and tell/build command warnings to
explicitly state that the two workflows are INCOMPATIBLE and cannot be mixed
- Add comprehensive BDD tests in features/plan_ulid_validation.feature
with step definitions using Typer CLI runner (not subprocess)
- Update CONTRIBUTING.md with 'Workflow Choice: Legacy vs. v3 Plan
Lifecycle' section documenting the incompatibility and migration path
ISSUES CLOSED: #1560
This commit is contained in:
@@ -1472,3 +1472,96 @@ def process_data(self, data: list[str], threshold: int) -> None:
|
|||||||
- Do not use bare `except:` or `except Exception:` without re-raising unless you have specific
|
- Do not use bare `except:` or `except Exception:` without re-raising unless you have specific
|
||||||
recovery logic.
|
recovery logic.
|
||||||
- Avoid returning `None` or default values when an error condition exists — raise exceptions.
|
- Avoid returning `None` or default values when an error condition exists — raise exceptions.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Workflow Choice: Legacy vs. v3 Plan Lifecycle
|
||||||
|
|
||||||
|
The CleverAgents CLI supports two distinct plan workflow systems. **These systems are mutually
|
||||||
|
exclusive and cannot be mixed.** Choosing one means committing to it for the entire lifecycle
|
||||||
|
of a plan.
|
||||||
|
|
||||||
|
### The Two Systems
|
||||||
|
|
||||||
|
| System | Commands | Identifier Format | Storage |
|
||||||
|
|--------|----------|-------------------|---------|
|
||||||
|
| **Legacy** (deprecated) | `agents tell`, `agents build`, `agents apply <name>` | Human-readable names (e.g., `"64-bit port plan"`) | `PlanService` (legacy) |
|
||||||
|
| **v3 Lifecycle** (authoritative) | `agents plan use`, `agents plan execute <PLAN_ID>`, `agents plan apply <PLAN_ID>` | ULIDs (e.g., `01HXM8C2ZK4Q7C2B3F2R4VYV6J`) | `PlanLifecycleService` |
|
||||||
|
|
||||||
|
### Why They Cannot Be Mixed
|
||||||
|
|
||||||
|
The two systems use **completely separate storage backends**. A plan created with a legacy
|
||||||
|
command (`agents tell`) exists only in the legacy `PlanService` storage. It is invisible to
|
||||||
|
v3 commands, which exclusively query `PlanLifecycleService`. Attempting to reference a legacy
|
||||||
|
plan name with a v3 command will always fail — not because the plan doesn't exist, but because
|
||||||
|
v3 commands look in a different storage system.
|
||||||
|
|
||||||
|
Additionally, v3 commands require **ULID identifiers** (26-character Crockford base32 strings).
|
||||||
|
Passing a human-readable name to `agents plan execute` or `agents plan apply` will be rejected
|
||||||
|
with an explicit error message explaining the incompatibility.
|
||||||
|
|
||||||
|
### The v3 Workflow (Recommended)
|
||||||
|
|
||||||
|
The v3 plan lifecycle is the authoritative workflow per `docs/specification.md`. All new
|
||||||
|
development should use v3 commands exclusively:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
# Step 1: Create a v3 plan from an action template
|
||||||
|
agents plan use local/my-action my-project
|
||||||
|
# Output: Plan ID: 01HXM8C2ZK4Q7C2B3F2R4VYV6J
|
||||||
|
|
||||||
|
# Step 2: Execute the plan (strategize + execute phases)
|
||||||
|
agents plan execute 01HXM8C2ZK4Q7C2B3F2R4VYV6J
|
||||||
|
|
||||||
|
# Step 3: Apply the changes
|
||||||
|
agents plan apply 01HXM8C2ZK4Q7C2B3F2R4VYV6J
|
||||||
|
```
|
||||||
|
|
||||||
|
### The Legacy Workflow (Deprecated)
|
||||||
|
|
||||||
|
Legacy commands are deprecated and will be removed in a future version. They emit deprecation
|
||||||
|
warnings when used. If you must use legacy commands, use them exclusively — do not attempt to
|
||||||
|
mix them with v3 commands:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
# Legacy workflow (deprecated — do not mix with v3 commands)
|
||||||
|
agents tell -n "my-plan" "Devise a plan to..."
|
||||||
|
agents build
|
||||||
|
agents apply my-plan
|
||||||
|
```
|
||||||
|
|
||||||
|
### Migration from Legacy to v3
|
||||||
|
|
||||||
|
There is no automatic migration path. Legacy plans cannot be converted to v3 plans. To migrate:
|
||||||
|
|
||||||
|
1. Identify what the legacy plan was intended to accomplish.
|
||||||
|
2. Create a new v3 plan using `agents plan use <action> <project>`.
|
||||||
|
3. Use `agents plan execute <PLAN_ID>` and `agents plan apply <PLAN_ID>` for subsequent steps.
|
||||||
|
|
||||||
|
### Error Messages
|
||||||
|
|
||||||
|
When a non-ULID identifier is passed to a v3 command, the CLI will display an explicit error
|
||||||
|
explaining the workflow incompatibility:
|
||||||
|
|
||||||
|
```
|
||||||
|
Error: Plan 'my-legacy-plan' not found.
|
||||||
|
|
||||||
|
The v3 plan lifecycle expects a ULID identifier
|
||||||
|
(e.g., 01HXM8C2ZK4Q7C2B3F2R4VYV6J), not a plan name.
|
||||||
|
|
||||||
|
Possible causes:
|
||||||
|
1. You created a plan with 'agents tell' (legacy workflow) and are
|
||||||
|
trying to reference it with a v3 command. These workflows are
|
||||||
|
incompatible and cannot be mixed — legacy plans exist only in the
|
||||||
|
legacy storage system and are invisible to v3 commands.
|
||||||
|
2. You referenced the wrong plan ID.
|
||||||
|
|
||||||
|
To use the v3 workflow:
|
||||||
|
- Run 'agents plan use <action> <project>' to create a v3 plan
|
||||||
|
(this returns a ULID you can use with subsequent commands).
|
||||||
|
- Run 'agents plan execute <PLAN_ID>' to execute it.
|
||||||
|
- Run 'agents plan apply <PLAN_ID>' to apply changes.
|
||||||
|
|
||||||
|
Legacy commands ('agents tell', 'agents build') operate in a separate
|
||||||
|
system and cannot be mixed with v3 commands.
|
||||||
|
```
|
||||||
|
|||||||
@@ -0,0 +1,141 @@
|
|||||||
|
Feature: ULID validation for v3 plan commands
|
||||||
|
As a CleverAgents user
|
||||||
|
I want clear, actionable error messages when I accidentally mix legacy and v3 plan workflows
|
||||||
|
So that I understand the incompatibility and know how to fix it
|
||||||
|
|
||||||
|
# ===================================================================
|
||||||
|
# _validate_plan_ulid unit tests
|
||||||
|
# ===================================================================
|
||||||
|
|
||||||
|
Scenario: _validate_plan_ulid accepts a valid ULID
|
||||||
|
When I call _validate_plan_ulid with "01HXM8C2ZK4Q7C2B3F2R4VYV6J"
|
||||||
|
Then ulid-val the validation should succeed and return "01HXM8C2ZK4Q7C2B3F2R4VYV6J"
|
||||||
|
|
||||||
|
Scenario: _validate_plan_ulid accepts a valid ULID in lowercase
|
||||||
|
When I call _validate_plan_ulid with "01hxm8c2zk4q7c2b3f2r4vyv6j"
|
||||||
|
Then ulid-val the validation should succeed and return "01hxm8c2zk4q7c2b3f2r4vyv6j"
|
||||||
|
|
||||||
|
Scenario: _validate_plan_ulid rejects a legacy plan name
|
||||||
|
When I call _validate_plan_ulid with "64-bit port plan"
|
||||||
|
Then ulid-val the validation should raise ValidationError
|
||||||
|
And ulid-val the error message should contain "not found"
|
||||||
|
And ulid-val the error message should contain "ULID"
|
||||||
|
And ulid-val the error message should contain "legacy"
|
||||||
|
And ulid-val the error message should contain "agents plan use"
|
||||||
|
|
||||||
|
Scenario: _validate_plan_ulid rejects a plain string that is not a ULID
|
||||||
|
When I call _validate_plan_ulid with "not-a-ulid"
|
||||||
|
Then ulid-val the validation should raise ValidationError
|
||||||
|
And ulid-val the error message should contain "ULID"
|
||||||
|
|
||||||
|
Scenario: _validate_plan_ulid rejects a string that is too short
|
||||||
|
When I call _validate_plan_ulid with "01ARZ3NDEK"
|
||||||
|
Then ulid-val the validation should raise ValidationError
|
||||||
|
And ulid-val the error message should contain "ULID"
|
||||||
|
|
||||||
|
Scenario: _validate_plan_ulid rejects a string that is too long
|
||||||
|
When I call _validate_plan_ulid with "01ARZ3NDEKTSV4RRFFQ69G5FAAEXTRA"
|
||||||
|
Then ulid-val the validation should raise ValidationError
|
||||||
|
And ulid-val the error message should contain "ULID"
|
||||||
|
|
||||||
|
Scenario: _validate_plan_ulid rejects a ULID with invalid characters (I, L, O, U)
|
||||||
|
When I call _validate_plan_ulid with "01ARZ3NDEKTSV4RRFFQ69G5FII"
|
||||||
|
Then ulid-val the validation should raise ValidationError
|
||||||
|
And ulid-val the error message should contain "ULID"
|
||||||
|
|
||||||
|
# ===================================================================
|
||||||
|
# agents plan execute — ULID validation
|
||||||
|
# ===================================================================
|
||||||
|
|
||||||
|
Scenario: plan execute rejects a legacy plan name with actionable error
|
||||||
|
Given a ulid-validation CLI runner
|
||||||
|
When I invoke ulid-validation plan execute with "my-legacy-plan"
|
||||||
|
Then the ulid-validation command should abort
|
||||||
|
And the ulid-validation output should contain "not found"
|
||||||
|
And the ulid-validation output should contain "ULID"
|
||||||
|
And the ulid-validation output should contain "legacy"
|
||||||
|
And the ulid-validation output should contain "agents plan use"
|
||||||
|
|
||||||
|
Scenario: plan execute rejects a non-ULID string with actionable error
|
||||||
|
Given a ulid-validation CLI runner
|
||||||
|
When I invoke ulid-validation plan execute with "not-a-valid-id"
|
||||||
|
Then the ulid-validation command should abort
|
||||||
|
And the ulid-validation output should contain "ULID"
|
||||||
|
|
||||||
|
Scenario: plan execute accepts a valid ULID and proceeds normally
|
||||||
|
Given a ulid-validation CLI runner
|
||||||
|
And a mocked lifecycle service for ulid-validation execute
|
||||||
|
When I invoke ulid-validation plan execute with "01HXM8C2ZK4Q7C2B3F2R4VYV6J"
|
||||||
|
Then the ulid-validation command should not abort due to ULID validation
|
||||||
|
|
||||||
|
# ===================================================================
|
||||||
|
# agents plan apply — ULID validation
|
||||||
|
# ===================================================================
|
||||||
|
|
||||||
|
Scenario: plan apply rejects a legacy plan name with actionable error
|
||||||
|
Given a ulid-validation CLI runner
|
||||||
|
When I invoke ulid-validation plan apply with "my-legacy-plan"
|
||||||
|
Then the ulid-validation command should abort
|
||||||
|
And the ulid-validation output should contain "not found"
|
||||||
|
And the ulid-validation output should contain "ULID"
|
||||||
|
And the ulid-validation output should contain "legacy"
|
||||||
|
|
||||||
|
Scenario: plan apply rejects a non-ULID string with actionable error
|
||||||
|
Given a ulid-validation CLI runner
|
||||||
|
When I invoke ulid-validation plan apply with "not-a-valid-id"
|
||||||
|
Then the ulid-validation command should abort
|
||||||
|
And the ulid-validation output should contain "ULID"
|
||||||
|
|
||||||
|
# ===================================================================
|
||||||
|
# agents plan status — ULID validation
|
||||||
|
# ===================================================================
|
||||||
|
|
||||||
|
Scenario: plan status rejects a legacy plan name with actionable error
|
||||||
|
Given a ulid-validation CLI runner
|
||||||
|
And a mocked lifecycle service for ulid-validation status
|
||||||
|
When I invoke ulid-validation plan status with "my-legacy-plan"
|
||||||
|
Then the ulid-validation command should abort
|
||||||
|
And the ulid-validation output should contain "ULID"
|
||||||
|
And the ulid-validation output should contain "legacy"
|
||||||
|
|
||||||
|
Scenario: plan status rejects a non-ULID string with actionable error
|
||||||
|
Given a ulid-validation CLI runner
|
||||||
|
And a mocked lifecycle service for ulid-validation status
|
||||||
|
When I invoke ulid-validation plan status with "not-a-valid-id"
|
||||||
|
Then the ulid-validation command should abort
|
||||||
|
And the ulid-validation output should contain "ULID"
|
||||||
|
|
||||||
|
# ===================================================================
|
||||||
|
# agents plan errors — ULID validation
|
||||||
|
# ===================================================================
|
||||||
|
|
||||||
|
Scenario: plan errors rejects a legacy plan name with actionable error
|
||||||
|
Given a ulid-validation CLI runner
|
||||||
|
When I invoke ulid-validation plan errors with "my-legacy-plan"
|
||||||
|
Then the ulid-validation command should abort
|
||||||
|
And the ulid-validation output should contain "ULID"
|
||||||
|
|
||||||
|
# ===================================================================
|
||||||
|
# agents plan cancel — ULID validation
|
||||||
|
# ===================================================================
|
||||||
|
|
||||||
|
Scenario: plan cancel rejects a legacy plan name with actionable error
|
||||||
|
Given a ulid-validation CLI runner
|
||||||
|
When I invoke ulid-validation plan cancel with "my-legacy-plan"
|
||||||
|
Then the ulid-validation command should abort
|
||||||
|
And the ulid-validation output should contain "ULID"
|
||||||
|
|
||||||
|
# ===================================================================
|
||||||
|
# Legacy deprecation warning content
|
||||||
|
# ===================================================================
|
||||||
|
|
||||||
|
Scenario: Legacy tell command deprecation warning explains workflow incompatibility
|
||||||
|
When I call tell_command programmatically for ulid-validation
|
||||||
|
Then the ulid-validation deprecation warning should mention "incompatible"
|
||||||
|
And the ulid-validation deprecation warning should mention "agents plan use"
|
||||||
|
And the ulid-validation deprecation warning should not suggest simple command swap
|
||||||
|
|
||||||
|
Scenario: Legacy build command deprecation warning explains workflow incompatibility
|
||||||
|
When I call build_command programmatically for ulid-validation
|
||||||
|
Then the ulid-validation deprecation warning should mention "incompatible"
|
||||||
|
And the ulid-validation deprecation warning should mention "agents plan use"
|
||||||
@@ -0,0 +1,359 @@
|
|||||||
|
"""Step definitions for plan_ulid_validation.feature.
|
||||||
|
|
||||||
|
Tests ULID format validation for v3 plan commands and the updated
|
||||||
|
legacy deprecation messages that explain workflow incompatibility.
|
||||||
|
|
||||||
|
All step text uses the ``ulid-validation`` prefix to avoid collisions
|
||||||
|
with other step definition files.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from datetime import datetime
|
||||||
|
from typing import Any
|
||||||
|
from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
|
from behave import given, then, when # type: ignore[import-untyped]
|
||||||
|
from typer.testing import CliRunner
|
||||||
|
|
||||||
|
from cleveragents.cli.commands.plan import app as plan_app
|
||||||
|
from cleveragents.core.exceptions import ValidationError
|
||||||
|
from cleveragents.domain.models.core.plan import (
|
||||||
|
NamespacedName,
|
||||||
|
Plan,
|
||||||
|
PlanIdentity,
|
||||||
|
PlanPhase,
|
||||||
|
PlanTimestamps,
|
||||||
|
ProcessingState,
|
||||||
|
)
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Helpers
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
_VALID_ULID = "01HXM8C2ZK4Q7C2B3F2R4VYV6J"
|
||||||
|
_runner = CliRunner()
|
||||||
|
|
||||||
|
|
||||||
|
def _make_plan(
|
||||||
|
*,
|
||||||
|
plan_id: str = _VALID_ULID,
|
||||||
|
name: str = "local/test-plan",
|
||||||
|
phase: PlanPhase = PlanPhase.EXECUTE,
|
||||||
|
processing_state: ProcessingState = ProcessingState.COMPLETE,
|
||||||
|
) -> Plan:
|
||||||
|
"""Create a minimal Plan instance for testing."""
|
||||||
|
timestamps = PlanTimestamps(
|
||||||
|
created_at=datetime.now(),
|
||||||
|
updated_at=datetime.now(),
|
||||||
|
)
|
||||||
|
return Plan(
|
||||||
|
identity=PlanIdentity(plan_id=plan_id),
|
||||||
|
namespaced_name=NamespacedName.parse(name),
|
||||||
|
action_name="local/test-action",
|
||||||
|
description="Test plan",
|
||||||
|
definition_of_done=None,
|
||||||
|
phase=phase,
|
||||||
|
processing_state=processing_state,
|
||||||
|
strategy_actor=None,
|
||||||
|
execution_actor=None,
|
||||||
|
project_links=[],
|
||||||
|
automation_profile=None,
|
||||||
|
invariants=[],
|
||||||
|
validation_summary=None,
|
||||||
|
error_message=None,
|
||||||
|
last_completed_step=-1,
|
||||||
|
last_checkpoint_id=None,
|
||||||
|
arguments={},
|
||||||
|
arguments_order=[],
|
||||||
|
estimation_actor=None,
|
||||||
|
invariant_actor=None,
|
||||||
|
timestamps=timestamps,
|
||||||
|
created_by=None,
|
||||||
|
reusable=True,
|
||||||
|
read_only=False,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# _validate_plan_ulid unit test steps
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
@when('I call _validate_plan_ulid with "{plan_id}"')
|
||||||
|
def step_call_validate_plan_ulid(context: Any, plan_id: str) -> None:
|
||||||
|
"""Call _validate_plan_ulid and capture result or exception."""
|
||||||
|
from cleveragents.cli.commands.plan import _validate_plan_ulid
|
||||||
|
|
||||||
|
context.ulid_validation_result = None
|
||||||
|
context.ulid_validation_error = None
|
||||||
|
try:
|
||||||
|
context.ulid_validation_result = _validate_plan_ulid(plan_id)
|
||||||
|
except ValidationError as exc:
|
||||||
|
context.ulid_validation_error = exc
|
||||||
|
|
||||||
|
|
||||||
|
@then('ulid-val the validation should succeed and return "{expected}"')
|
||||||
|
def step_validation_succeeds(context: Any, expected: str) -> None:
|
||||||
|
"""Assert that validation succeeded and returned the expected value."""
|
||||||
|
assert context.ulid_validation_error is None, (
|
||||||
|
f"Expected success but got ValidationError: {context.ulid_validation_error}"
|
||||||
|
)
|
||||||
|
assert context.ulid_validation_result == expected, (
|
||||||
|
f"Expected '{expected}' but got '{context.ulid_validation_result}'"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@then("ulid-val the validation should raise ValidationError")
|
||||||
|
def step_validation_raises(context: Any) -> None:
|
||||||
|
"""Assert that validation raised a ValidationError."""
|
||||||
|
assert context.ulid_validation_error is not None, (
|
||||||
|
"Expected ValidationError but validation succeeded with result: "
|
||||||
|
f"'{context.ulid_validation_result}'"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@then('ulid-val the error message should contain "{text}"')
|
||||||
|
def step_error_message_contains(context: Any, text: str) -> None:
|
||||||
|
"""Assert that the ValidationError message contains the given text."""
|
||||||
|
assert context.ulid_validation_error is not None, (
|
||||||
|
"No ValidationError was raised — cannot check error message."
|
||||||
|
)
|
||||||
|
error_msg = context.ulid_validation_error.message
|
||||||
|
assert text in error_msg, (
|
||||||
|
f"Expected error message to contain '{text}' but got:\n{error_msg}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# CLI runner setup
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
@given("a ulid-validation CLI runner")
|
||||||
|
def step_ulid_validation_runner(context: Any) -> None:
|
||||||
|
"""Set up a CLI runner and clean up patches after the scenario."""
|
||||||
|
context.ulid_runner = _runner
|
||||||
|
context.ulid_result = None
|
||||||
|
context.ulid_cleanups = [] # list[Any]
|
||||||
|
|
||||||
|
def _cleanup(ctx: Any) -> None:
|
||||||
|
for stop in getattr(ctx, "ulid_cleanups", []):
|
||||||
|
stop()
|
||||||
|
|
||||||
|
context.add_cleanup(_cleanup, context)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Mocked service setup steps
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
@given("a mocked lifecycle service for ulid-validation execute")
|
||||||
|
def step_mock_service_execute(context: Any) -> None:
|
||||||
|
"""Patch the lifecycle service and executor for execute_plan tests."""
|
||||||
|
mock_svc = MagicMock()
|
||||||
|
mock_executor = MagicMock()
|
||||||
|
|
||||||
|
plan = _make_plan(
|
||||||
|
plan_id=_VALID_ULID,
|
||||||
|
phase=PlanPhase.STRATEGIZE,
|
||||||
|
processing_state=ProcessingState.QUEUED,
|
||||||
|
)
|
||||||
|
mock_svc.get_plan.return_value = plan
|
||||||
|
mock_svc.list_plans.return_value = [plan]
|
||||||
|
mock_executor.run_strategize.return_value = None
|
||||||
|
|
||||||
|
p1 = patch(
|
||||||
|
"cleveragents.cli.commands.plan._get_lifecycle_service",
|
||||||
|
return_value=mock_svc,
|
||||||
|
)
|
||||||
|
p2 = patch(
|
||||||
|
"cleveragents.cli.commands.plan._get_plan_executor",
|
||||||
|
return_value=mock_executor,
|
||||||
|
)
|
||||||
|
p1.start()
|
||||||
|
p2.start()
|
||||||
|
context.ulid_cleanups.extend([p1.stop, p2.stop])
|
||||||
|
context.ulid_mock_svc = mock_svc
|
||||||
|
|
||||||
|
|
||||||
|
@given("a mocked lifecycle service for ulid-validation status")
|
||||||
|
def step_mock_service_status(context: Any) -> None:
|
||||||
|
"""Patch the lifecycle service for plan_status tests."""
|
||||||
|
mock_svc = MagicMock()
|
||||||
|
mock_svc.list_plans.return_value = []
|
||||||
|
|
||||||
|
p = patch(
|
||||||
|
"cleveragents.cli.commands.plan._get_lifecycle_service",
|
||||||
|
return_value=mock_svc,
|
||||||
|
)
|
||||||
|
p.start()
|
||||||
|
context.ulid_cleanups.append(p.stop)
|
||||||
|
context.ulid_mock_svc = mock_svc
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# CLI invocation steps — execute
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
@when('I invoke ulid-validation plan execute with "{plan_id}"')
|
||||||
|
def step_invoke_execute(context: Any, plan_id: str) -> None:
|
||||||
|
"""Invoke 'agents plan execute <plan_id>' via the CLI runner."""
|
||||||
|
context.ulid_result = context.ulid_runner.invoke(
|
||||||
|
plan_app, ["execute", plan_id], catch_exceptions=False
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# CLI invocation steps — apply
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
@when('I invoke ulid-validation plan apply with "{plan_id}"')
|
||||||
|
def step_invoke_apply(context: Any, plan_id: str) -> None:
|
||||||
|
"""Invoke 'agents plan apply <plan_id>' via the CLI runner."""
|
||||||
|
context.ulid_result = context.ulid_runner.invoke(
|
||||||
|
plan_app, ["apply", plan_id], catch_exceptions=False
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# CLI invocation steps — status
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
@when('I invoke ulid-validation plan status with "{plan_id}"')
|
||||||
|
def step_invoke_status(context: Any, plan_id: str) -> None:
|
||||||
|
"""Invoke 'agents plan status <plan_id>' via the CLI runner."""
|
||||||
|
context.ulid_result = context.ulid_runner.invoke(
|
||||||
|
plan_app, ["status", plan_id], catch_exceptions=False
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# CLI invocation steps — errors
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
@when('I invoke ulid-validation plan errors with "{plan_id}"')
|
||||||
|
def step_invoke_errors(context: Any, plan_id: str) -> None:
|
||||||
|
"""Invoke 'agents plan errors <plan_id>' via the CLI runner."""
|
||||||
|
context.ulid_result = context.ulid_runner.invoke(
|
||||||
|
plan_app, ["errors", plan_id], catch_exceptions=False
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# CLI invocation steps — cancel
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
@when('I invoke ulid-validation plan cancel with "{plan_id}"')
|
||||||
|
def step_invoke_cancel(context: Any, plan_id: str) -> None:
|
||||||
|
"""Invoke 'agents plan cancel <plan_id>' via the CLI runner."""
|
||||||
|
context.ulid_result = context.ulid_runner.invoke(
|
||||||
|
plan_app, ["cancel", plan_id], catch_exceptions=False
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# CLI assertion steps
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
@then("the ulid-validation command should abort")
|
||||||
|
def step_command_aborts(context: Any) -> None:
|
||||||
|
"""Assert that the CLI command exited with a non-zero exit code."""
|
||||||
|
assert context.ulid_result is not None, "No CLI result captured."
|
||||||
|
assert context.ulid_result.exit_code != 0, (
|
||||||
|
f"Expected non-zero exit code but got {context.ulid_result.exit_code}.\n"
|
||||||
|
f"Output: {context.ulid_result.output}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@then("the ulid-validation command should not abort due to ULID validation")
|
||||||
|
def step_command_does_not_abort_on_ulid(context: Any) -> None:
|
||||||
|
"""Assert that the CLI command did not abort due to ULID validation.
|
||||||
|
|
||||||
|
The command may still abort for other reasons (e.g., plan not found
|
||||||
|
in the mocked service), but it should not abort with a ULID error.
|
||||||
|
"""
|
||||||
|
assert context.ulid_result is not None, "No CLI result captured."
|
||||||
|
output = context.ulid_result.output
|
||||||
|
# If it aborted, it should NOT be because of ULID validation
|
||||||
|
if context.ulid_result.exit_code != 0:
|
||||||
|
assert "ULID" not in output or "not found" not in output, (
|
||||||
|
f"Command aborted due to ULID validation when it should have passed.\n"
|
||||||
|
f"Output: {output}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@then('the ulid-validation output should contain "{text}"')
|
||||||
|
def step_output_contains(context: Any, text: str) -> None:
|
||||||
|
"""Assert that the CLI output contains the given text."""
|
||||||
|
assert context.ulid_result is not None, "No CLI result captured."
|
||||||
|
output = context.ulid_result.output
|
||||||
|
assert text in output, f"Expected output to contain '{text}' but got:\n{output}"
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Legacy deprecation warning steps
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
@when("I call tell_command programmatically for ulid-validation")
|
||||||
|
def step_call_tell_command(context: Any) -> None:
|
||||||
|
"""Invoke the legacy tell command via CLI runner and capture its output.
|
||||||
|
|
||||||
|
The legacy commands emit deprecation warnings via ``console.print``, not
|
||||||
|
via Python's ``warnings.warn``. We therefore use the Typer CLI runner to
|
||||||
|
capture the printed output and store it for assertion steps.
|
||||||
|
"""
|
||||||
|
runner = CliRunner()
|
||||||
|
result = runner.invoke(plan_app, ["tell", "test prompt"], catch_exceptions=False)
|
||||||
|
context.ulid_deprecation_warnings = [result.output]
|
||||||
|
|
||||||
|
|
||||||
|
@when("I call build_command programmatically for ulid-validation")
|
||||||
|
def step_call_build_command(context: Any) -> None:
|
||||||
|
"""Invoke the legacy build command via CLI runner and capture its output.
|
||||||
|
|
||||||
|
The legacy commands emit deprecation warnings via ``console.print``, not
|
||||||
|
via Python's ``warnings.warn``. We therefore use the Typer CLI runner to
|
||||||
|
capture the printed output and store it for assertion steps.
|
||||||
|
"""
|
||||||
|
runner = CliRunner()
|
||||||
|
result = runner.invoke(plan_app, ["build"], catch_exceptions=False)
|
||||||
|
context.ulid_deprecation_warnings = [result.output]
|
||||||
|
|
||||||
|
|
||||||
|
@then('the ulid-validation deprecation warning should mention "{text}"')
|
||||||
|
def step_deprecation_mentions(context: Any, text: str) -> None:
|
||||||
|
"""Assert that the CLI output mentions the given text (case-insensitive)."""
|
||||||
|
assert context.ulid_deprecation_warnings, "No CLI output was captured."
|
||||||
|
combined = " ".join(context.ulid_deprecation_warnings).lower()
|
||||||
|
assert text.lower() in combined, (
|
||||||
|
f"Expected CLI output to mention '{text}' (case-insensitive) but got:\n{combined}"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@then("the ulid-validation deprecation warning should not suggest simple command swap")
|
||||||
|
def step_deprecation_no_simple_swap(context: Any) -> None:
|
||||||
|
"""Assert that the deprecation output does not suggest a 1:1 command swap.
|
||||||
|
|
||||||
|
The old message said "Use 'agents plan use <action> [project]' for the v3
|
||||||
|
lifecycle" which implied a simple substitution. The new message must
|
||||||
|
explain the workflow incompatibility instead.
|
||||||
|
"""
|
||||||
|
assert context.ulid_deprecation_warnings, "No CLI output was captured."
|
||||||
|
combined = " ".join(context.ulid_deprecation_warnings)
|
||||||
|
# The new message must NOT contain the old misleading phrasing
|
||||||
|
old_misleading_phrase = (
|
||||||
|
"Use 'agents plan use <action> [project]' for the v3 lifecycle."
|
||||||
|
)
|
||||||
|
assert old_misleading_phrase not in combined, (
|
||||||
|
f"Deprecation output still contains the misleading 1:1 swap suggestion:\n"
|
||||||
|
f"{combined}"
|
||||||
|
)
|
||||||
@@ -48,6 +48,29 @@ from cleveragents.domain.models.core.plan import PlanPhase, ProcessingState
|
|||||||
# Regex for validating namespaced actor names (namespace/name format)
|
# Regex for validating namespaced actor names (namespace/name format)
|
||||||
_NAMESPACED_ACTOR_RE = re.compile(r"^[a-z][a-z0-9-]*/[a-z][a-z0-9._-]*$")
|
_NAMESPACED_ACTOR_RE = re.compile(r"^[a-z][a-z0-9-]*/[a-z][a-z0-9._-]*$")
|
||||||
|
|
||||||
|
# Regex for validating ULID format (26 characters, Crockford base32 alphabet)
|
||||||
|
# ULIDs use the alphabet: 0123456789ABCDEFGHJKMNPQRSTVWXYZ (case-insensitive)
|
||||||
|
_PLAN_ULID_RE = re.compile(r"^[0-9A-HJKMNP-TV-Z]{26}$", re.IGNORECASE)
|
||||||
|
|
||||||
|
# Actionable error message shown when a non-ULID is passed to a v3 command
|
||||||
|
_ULID_VALIDATION_ERROR_MSG = (
|
||||||
|
"The v3 plan lifecycle expects a ULID identifier "
|
||||||
|
"(e.g., 01HXM8C2ZK4Q7C2B3F2R4VYV6J), not a plan name.\n\n"
|
||||||
|
"Possible causes:\n"
|
||||||
|
" 1. You created a plan with 'agents tell' (legacy workflow) and are\n"
|
||||||
|
" trying to reference it with a v3 command. These workflows are\n"
|
||||||
|
" incompatible and cannot be mixed — legacy plans exist only in the\n"
|
||||||
|
" legacy storage system and are invisible to v3 commands.\n"
|
||||||
|
" 2. You referenced the wrong plan ID.\n\n"
|
||||||
|
"To use the v3 workflow:\n"
|
||||||
|
" - Run 'agents plan use <action> <project>' to create a v3 plan\n"
|
||||||
|
" (this returns a ULID you can use with subsequent commands).\n"
|
||||||
|
" - Run 'agents plan execute <PLAN_ID>' to execute it.\n"
|
||||||
|
" - Run 'agents plan apply <PLAN_ID>' to apply changes.\n\n"
|
||||||
|
"Legacy commands ('agents tell', 'agents build') operate in a separate\n"
|
||||||
|
"system and cannot be mixed with v3 commands."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _notify_facade(operation: str, params: dict[str, Any]) -> None:
|
def _notify_facade(operation: str, params: dict[str, Any]) -> None:
|
||||||
"""Best-effort A2A facade notification for protocol bookkeeping.
|
"""Best-effort A2A facade notification for protocol bookkeeping.
|
||||||
@@ -99,9 +122,46 @@ def validate_namespaced_actor(value: str, flag_name: str) -> str:
|
|||||||
return value
|
return value
|
||||||
|
|
||||||
|
|
||||||
|
def _validate_plan_ulid(plan_id: str) -> str:
|
||||||
|
"""Validate that ``plan_id`` is a valid ULID before querying v3 storage.
|
||||||
|
|
||||||
|
The v3 plan lifecycle exclusively uses ULIDs as plan identifiers.
|
||||||
|
Passing a human-readable name (e.g., from the legacy ``agents tell``
|
||||||
|
workflow) will always fail with a confusing "Plan not found" error
|
||||||
|
because the two systems use completely separate storage.
|
||||||
|
|
||||||
|
This function detects that mismatch early and provides an actionable
|
||||||
|
error message explaining the workflow incompatibility.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
plan_id: The plan identifier string to validate.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
The validated ``plan_id`` (unchanged) if it is a valid ULID.
|
||||||
|
|
||||||
|
Raises:
|
||||||
|
ValidationError: If ``plan_id`` does not match the ULID format,
|
||||||
|
with a message explaining the legacy/v3 workflow incompatibility
|
||||||
|
and the correct migration path.
|
||||||
|
"""
|
||||||
|
if not _PLAN_ULID_RE.match(plan_id):
|
||||||
|
raise ValidationError(
|
||||||
|
f"Plan '{plan_id}' not found.\n\n" + _ULID_VALIDATION_ERROR_MSG
|
||||||
|
)
|
||||||
|
return plan_id
|
||||||
|
|
||||||
|
|
||||||
_LEGACY_DEPRECATION_MSG = (
|
_LEGACY_DEPRECATION_MSG = (
|
||||||
"This command uses the legacy plan workflow and is deprecated. "
|
"This command uses the legacy plan workflow and is deprecated.\n"
|
||||||
"Use 'agents plan use <action> [project]' for the v3 lifecycle."
|
"WARNING: The legacy and v3 plan workflows are INCOMPATIBLE and cannot\n"
|
||||||
|
"be mixed. Plans created with legacy commands ('agents tell', 'agents build')\n"
|
||||||
|
"exist only in the legacy storage system and cannot be referenced by v3\n"
|
||||||
|
"commands ('agents plan execute', 'agents plan apply').\n\n"
|
||||||
|
"To migrate to the v3 workflow:\n"
|
||||||
|
" 1. Use 'agents plan use <action> <project>' to create a new v3 plan.\n"
|
||||||
|
" 2. Use 'agents plan execute <PLAN_ID>' to execute it.\n"
|
||||||
|
" 3. Use 'agents plan apply <PLAN_ID>' to apply changes.\n\n"
|
||||||
|
"Do NOT attempt to use a legacy plan name with v3 commands — it will fail."
|
||||||
)
|
)
|
||||||
|
|
||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
@@ -615,8 +675,12 @@ def tell(
|
|||||||
from cleveragents.application.services.plan_service import PlanService
|
from cleveragents.application.services.plan_service import PlanService
|
||||||
|
|
||||||
console.print(
|
console.print(
|
||||||
"[yellow]Warning:[/yellow] 'tell' is a legacy command. "
|
"[yellow]Warning:[/yellow] 'tell' is a legacy command and is deprecated.\n"
|
||||||
"Use 'agents plan use <action> [project]' for the v3 lifecycle."
|
"[yellow]WARNING:[/yellow] The legacy and v3 plan workflows are INCOMPATIBLE "
|
||||||
|
"and cannot be mixed.\n"
|
||||||
|
"Plans created here cannot be referenced by v3 commands "
|
||||||
|
"('agents plan execute', 'agents plan apply').\n"
|
||||||
|
"To use the v3 workflow: 'agents plan use <action> <project>'"
|
||||||
)
|
)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
@@ -715,8 +779,11 @@ def build(
|
|||||||
from cleveragents.application.services.plan_service import PlanService
|
from cleveragents.application.services.plan_service import PlanService
|
||||||
|
|
||||||
console.print(
|
console.print(
|
||||||
"[yellow]Warning:[/yellow] 'build' is a legacy command. "
|
"[yellow]Warning:[/yellow] 'build' is a legacy command and is deprecated.\n"
|
||||||
"Use 'agents plan execute <plan_id>' for the v3 lifecycle."
|
"[yellow]WARNING:[/yellow] The legacy and v3 plan workflows are INCOMPATIBLE "
|
||||||
|
"and cannot be mixed.\n"
|
||||||
|
"Plans created here cannot be referenced by v3 commands.\n"
|
||||||
|
"To use the v3 workflow: 'agents plan use <action> <project>'"
|
||||||
)
|
)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
@@ -795,6 +862,12 @@ def _lifecycle_apply_with_id(plan_id: str, fmt: str = "rich") -> None:
|
|||||||
)
|
)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
# Validate ULID format before querying v3 storage. A non-ULID
|
||||||
|
# identifier (e.g., a legacy plan name) will never be found in v3
|
||||||
|
# storage; catching it here provides an actionable error message
|
||||||
|
# instead of a generic "Plan not found".
|
||||||
|
_validate_plan_ulid(plan_id)
|
||||||
|
|
||||||
service = _get_lifecycle_service()
|
service = _get_lifecycle_service()
|
||||||
|
|
||||||
# Fail-fast: read-only plans must not enter Apply phase
|
# Fail-fast: read-only plans must not enter Apply phase
|
||||||
@@ -1733,6 +1806,12 @@ def execute_plan(
|
|||||||
)
|
)
|
||||||
raise typer.Abort()
|
raise typer.Abort()
|
||||||
plan_id = eligible[0].identity.plan_id
|
plan_id = eligible[0].identity.plan_id
|
||||||
|
else:
|
||||||
|
# Validate ULID format before querying v3 storage. A non-ULID
|
||||||
|
# identifier (e.g., a legacy plan name) will never be found in v3
|
||||||
|
# storage; catching it here provides an actionable error message
|
||||||
|
# instead of a generic "Plan not found".
|
||||||
|
_validate_plan_ulid(plan_id)
|
||||||
|
|
||||||
# Apply execution environment override if provided
|
# Apply execution environment override if provided
|
||||||
if execution_environment:
|
if execution_environment:
|
||||||
@@ -1922,6 +2001,12 @@ def lifecycle_apply_plan(
|
|||||||
console.print(f" • {p.identity.plan_id}: {p.namespaced_name}")
|
console.print(f" • {p.identity.plan_id}: {p.namespaced_name}")
|
||||||
raise typer.Abort()
|
raise typer.Abort()
|
||||||
plan_id = eligible_plans[0].identity.plan_id
|
plan_id = eligible_plans[0].identity.plan_id
|
||||||
|
else:
|
||||||
|
# Validate ULID format before querying v3 storage. A non-ULID
|
||||||
|
# identifier (e.g., a legacy plan name) will never be found in v3
|
||||||
|
# storage; catching it here provides an actionable error message
|
||||||
|
# instead of a generic "Plan not found".
|
||||||
|
_validate_plan_ulid(plan_id)
|
||||||
|
|
||||||
# Fail-fast: plan must exist
|
# Fail-fast: plan must exist
|
||||||
pre_plan = service.get_plan(plan_id)
|
pre_plan = service.get_plan(plan_id)
|
||||||
@@ -2073,6 +2158,12 @@ def plan_status(
|
|||||||
console.print(table)
|
console.print(table)
|
||||||
return
|
return
|
||||||
|
|
||||||
|
# Validate ULID format before querying v3 storage. A non-ULID
|
||||||
|
# identifier (e.g., a legacy plan name) will never be found in v3
|
||||||
|
# storage; catching it here provides an actionable error message
|
||||||
|
# instead of a generic "Plan not found".
|
||||||
|
_validate_plan_ulid(plan_id)
|
||||||
|
|
||||||
# Show single plan details
|
# Show single plan details
|
||||||
plan = service.get_plan(plan_id)
|
plan = service.get_plan(plan_id)
|
||||||
|
|
||||||
@@ -2115,6 +2206,7 @@ def plan_errors(
|
|||||||
)
|
)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
_validate_plan_ulid(plan_id)
|
||||||
service = _get_lifecycle_service()
|
service = _get_lifecycle_service()
|
||||||
plan = service.get_plan(plan_id)
|
plan = service.get_plan(plan_id)
|
||||||
|
|
||||||
@@ -2414,9 +2506,7 @@ def lifecycle_list_plans(
|
|||||||
completed_count = sum(
|
completed_count = sum(
|
||||||
1 for p in plans if p.processing_state.value == "complete"
|
1 for p in plans if p.processing_state.value == "complete"
|
||||||
)
|
)
|
||||||
errored_count = sum(
|
errored_count = sum(1 for p in plans if p.processing_state.value == "errored")
|
||||||
1 for p in plans if p.processing_state.value == "errored"
|
|
||||||
)
|
|
||||||
|
|
||||||
summary_text = (
|
summary_text = (
|
||||||
f"[yellow bold]Total:[/yellow bold] {total_plans}\n"
|
f"[yellow bold]Total:[/yellow bold] {total_plans}\n"
|
||||||
@@ -2429,10 +2519,7 @@ def lifecycle_list_plans(
|
|||||||
|
|
||||||
# Print success message
|
# Print success message
|
||||||
plan_word = "plan" if total_plans == 1 else "plans"
|
plan_word = "plan" if total_plans == 1 else "plans"
|
||||||
console.print(
|
console.print(f"[green bold]✓ OK[/green bold] {total_plans} {plan_word} listed")
|
||||||
f"[green bold]✓ OK[/green bold] {total_plans} {plan_word} listed"
|
|
||||||
)
|
|
||||||
|
|
||||||
|
|
||||||
except CleverAgentsError as e:
|
except CleverAgentsError as e:
|
||||||
console.print(f"[red]Error:[/red] {e.message}")
|
console.print(f"[red]Error:[/red] {e.message}")
|
||||||
@@ -2467,6 +2554,7 @@ def cancel_plan(
|
|||||||
Plans in terminal state cannot be cancelled.
|
Plans in terminal state cannot be cancelled.
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
|
_validate_plan_ulid(plan_id)
|
||||||
service = _get_lifecycle_service()
|
service = _get_lifecycle_service()
|
||||||
|
|
||||||
plan = service.cancel_plan(plan_id, reason=reason)
|
plan = service.cancel_plan(plan_id, reason=reason)
|
||||||
|
|||||||
Reference in New Issue
Block a user