forked from cleveragents/cleveragents-core
Merge pull request 'fix(cli): disallow mixing legacy and v3 plan workflows' (#1577) from fix/prevent-legacy-v3-mixing into master
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
|
||||
recovery logic.
|
||||
- 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)
|
||||
_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:
|
||||
"""Best-effort A2A facade notification for protocol bookkeeping.
|
||||
@@ -99,9 +122,46 @@ def validate_namespaced_actor(value: str, flag_name: str) -> str:
|
||||
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 = (
|
||||
"This command uses the legacy plan workflow and is deprecated. "
|
||||
"Use 'agents plan use <action> [project]' for the v3 lifecycle."
|
||||
"This command uses the legacy plan workflow and is deprecated.\n"
|
||||
"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:
|
||||
@@ -615,8 +675,12 @@ def tell(
|
||||
from cleveragents.application.services.plan_service import PlanService
|
||||
|
||||
console.print(
|
||||
"[yellow]Warning:[/yellow] 'tell' is a legacy command. "
|
||||
"Use 'agents plan use <action> [project]' for the v3 lifecycle."
|
||||
"[yellow]Warning:[/yellow] 'tell' is a legacy command and is deprecated.\n"
|
||||
"[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:
|
||||
@@ -715,8 +779,11 @@ def build(
|
||||
from cleveragents.application.services.plan_service import PlanService
|
||||
|
||||
console.print(
|
||||
"[yellow]Warning:[/yellow] 'build' is a legacy command. "
|
||||
"Use 'agents plan execute <plan_id>' for the v3 lifecycle."
|
||||
"[yellow]Warning:[/yellow] 'build' is a legacy command and is deprecated.\n"
|
||||
"[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:
|
||||
@@ -795,6 +862,12 @@ def _lifecycle_apply_with_id(plan_id: str, fmt: str = "rich") -> None:
|
||||
)
|
||||
|
||||
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()
|
||||
|
||||
# Fail-fast: read-only plans must not enter Apply phase
|
||||
@@ -1733,6 +1806,12 @@ def execute_plan(
|
||||
)
|
||||
raise typer.Abort()
|
||||
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
|
||||
if execution_environment:
|
||||
@@ -1922,6 +2001,12 @@ def lifecycle_apply_plan(
|
||||
console.print(f" • {p.identity.plan_id}: {p.namespaced_name}")
|
||||
raise typer.Abort()
|
||||
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
|
||||
pre_plan = service.get_plan(plan_id)
|
||||
@@ -2073,6 +2158,12 @@ def plan_status(
|
||||
console.print(table)
|
||||
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
|
||||
plan = service.get_plan(plan_id)
|
||||
|
||||
@@ -2115,6 +2206,7 @@ def plan_errors(
|
||||
)
|
||||
|
||||
try:
|
||||
_validate_plan_ulid(plan_id)
|
||||
service = _get_lifecycle_service()
|
||||
plan = service.get_plan(plan_id)
|
||||
|
||||
@@ -2414,9 +2506,7 @@ def lifecycle_list_plans(
|
||||
completed_count = sum(
|
||||
1 for p in plans if p.processing_state.value == "complete"
|
||||
)
|
||||
errored_count = sum(
|
||||
1 for p in plans if p.processing_state.value == "errored"
|
||||
)
|
||||
errored_count = sum(1 for p in plans if p.processing_state.value == "errored")
|
||||
|
||||
summary_text = (
|
||||
f"[yellow bold]Total:[/yellow bold] {total_plans}\n"
|
||||
@@ -2429,10 +2519,7 @@ def lifecycle_list_plans(
|
||||
|
||||
# Print success message
|
||||
plan_word = "plan" if total_plans == 1 else "plans"
|
||||
console.print(
|
||||
f"[green bold]✓ OK[/green bold] {total_plans} {plan_word} listed"
|
||||
)
|
||||
|
||||
console.print(f"[green bold]✓ OK[/green bold] {total_plans} {plan_word} listed")
|
||||
|
||||
except CleverAgentsError as e:
|
||||
console.print(f"[red]Error:[/red] {e.message}")
|
||||
@@ -2467,6 +2554,7 @@ def cancel_plan(
|
||||
Plans in terminal state cannot be cancelled.
|
||||
"""
|
||||
try:
|
||||
_validate_plan_ulid(plan_id)
|
||||
service = _get_lifecycle_service()
|
||||
|
||||
plan = service.cancel_plan(plan_id, reason=reason)
|
||||
|
||||
Reference in New Issue
Block a user