cleveragents/cleveragents-core

Fork 3

UAT: `agents plan rollback` accepts optional checkpoint_id but spec requires it as mandatory positional argument #5463

New issue

Open

opened 2026-04-09 06:55:44 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-09 06:55:44 +00:00

Owner

Summary

The agents plan rollback command makes checkpoint_id optional (defaulting to None) and falls back to --to-checkpoint option. The specification defines <CHECKPOINT_ID> as a required positional argument. This means users can invoke agents plan rollback <PLAN_ID> without a checkpoint and get a runtime error instead of a clear usage error.

Expected Behavior (from spec)

agents plan rollback [--yes|-y] <PLAN_ID> <CHECKPOINT_ID>

Both <PLAN_ID> and <CHECKPOINT_ID> are required positional arguments. Omitting <CHECKPOINT_ID> should produce a usage error immediately (exit code 2), not a runtime error.

Actual Behavior

# src/cleveragents/cli/commands/plan.py lines 3532-3594
@app.command("rollback")
def rollback_plan(
    plan_id: Annotated[str, typer.Argument(...)],
    checkpoint_id: Annotated[
        str | None,
        typer.Argument(help="Checkpoint ID to restore (positional)"),
    ] = None,  # ← Optional! Should be required
    to_checkpoint: Annotated[
        str | None,
        typer.Option("--to-checkpoint", ...),
    ] = None,
    ...

When checkpoint_id is None and to_checkpoint is also None, the code prints:

Error: checkpoint ID required. Provide as positional argument or via --to-checkpoint.

and calls raise typer.Abort() — which exits with code 1 (general error), not code 2 (usage error).

Problems

checkpoint_id should be required — the spec shows it as a mandatory positional argument <CHECKPOINT_ID>, not optional
--to-checkpoint is not in the spec — the spec only shows the positional form; the --to-checkpoint option is an undocumented extension
Wrong exit code — missing required argument should exit with code 2 (EXIT_USAGE), not code 1 (EXIT_ERROR) via typer.Abort()

Code Location

File: src/cleveragents/cli/commands/plan.py
Function: rollback_plan() at line 3532
Issue: checkpoint_id parameter has = None default making it optional

Spec Reference

docs/specification.md line 16042:

agents plan rollback [--yes|-y] <PLAN_ID> <CHECKPOINT_ID>

Impact

Users who forget the checkpoint ID get a confusing runtime error instead of a clear "missing argument" usage error
The --to-checkpoint option creates an undocumented API surface not in the spec
Wrong exit code (1 vs 2) breaks scripts that check exit codes for usage errors

Subtasks

Make checkpoint_id a required positional argument (remove = None default)
Remove or deprecate --to-checkpoint option (not in spec)
Ensure missing checkpoint_id exits with code 2 (EXIT_USAGE)
Update help text to match spec signature
Add unit tests for missing argument error case

Definition of Done

agents plan rollback <PLAN_ID> (without checkpoint) exits with code 2 and clear usage error
agents plan rollback <PLAN_ID> <CHECKPOINT_ID> works correctly
Exit code 2 for usage errors, 0 for success, 1 for runtime errors

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: uat-tester

## Summary The `agents plan rollback` command makes `checkpoint_id` optional (defaulting to `None`) and falls back to `--to-checkpoint` option. The specification defines `<CHECKPOINT_ID>` as a required positional argument. This means users can invoke `agents plan rollback <PLAN_ID>` without a checkpoint and get a runtime error instead of a clear usage error. ## Expected Behavior (from spec) ``` agents plan rollback [--yes|-y] <PLAN_ID> <CHECKPOINT_ID> ``` Both `<PLAN_ID>` and `<CHECKPOINT_ID>` are required positional arguments. Omitting `<CHECKPOINT_ID>` should produce a usage error immediately (exit code 2), not a runtime error. ## Actual Behavior ```python # src/cleveragents/cli/commands/plan.py lines 3532-3594 @app.command("rollback") def rollback_plan( plan_id: Annotated[str, typer.Argument(...)], checkpoint_id: Annotated[ str | None, typer.Argument(help="Checkpoint ID to restore (positional)"), ] = None, # ← Optional! Should be required to_checkpoint: Annotated[ str | None, typer.Option("--to-checkpoint", ...), ] = None, ... ``` When `checkpoint_id` is `None` and `to_checkpoint` is also `None`, the code prints: ``` Error: checkpoint ID required. Provide as positional argument or via --to-checkpoint. ``` and calls `raise typer.Abort()` — which exits with code 1 (general error), not code 2 (usage error). ## Problems 1. **`checkpoint_id` should be required** — the spec shows it as a mandatory positional argument `<CHECKPOINT_ID>`, not optional 2. **`--to-checkpoint` is not in the spec** — the spec only shows the positional form; the `--to-checkpoint` option is an undocumented extension 3. **Wrong exit code** — missing required argument should exit with code 2 (EXIT_USAGE), not code 1 (EXIT_ERROR) via `typer.Abort()` ## Code Location - **File**: `src/cleveragents/cli/commands/plan.py` - **Function**: `rollback_plan()` at line 3532 - **Issue**: `checkpoint_id` parameter has `= None` default making it optional ## Spec Reference - `docs/specification.md` line 16042: ``` agents plan rollback [--yes|-y] <PLAN_ID> <CHECKPOINT_ID> ``` ## Impact - Users who forget the checkpoint ID get a confusing runtime error instead of a clear "missing argument" usage error - The `--to-checkpoint` option creates an undocumented API surface not in the spec - Wrong exit code (1 vs 2) breaks scripts that check exit codes for usage errors ## Subtasks - [ ] Make `checkpoint_id` a required positional argument (remove `= None` default) - [ ] Remove or deprecate `--to-checkpoint` option (not in spec) - [ ] Ensure missing `checkpoint_id` exits with code 2 (EXIT_USAGE) - [ ] Update help text to match spec signature - [ ] Add unit tests for missing argument error case ## Definition of Done - `agents plan rollback <PLAN_ID>` (without checkpoint) exits with code 2 and clear usage error - `agents plan rollback <PLAN_ID> <CHECKPOINT_ID>` works correctly - Exit code 2 for usage errors, 0 for success, 1 for runtime errors --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: uat-tester

HAL9000 added the

labels

2026-04-09 06:56:03 +00:00

HAL9000 added this to the v3.3.0 milestone

2026-04-09 06:59:55 +00:00

HAL9000 added

and removed

labels

2026-04-09 07:00:17 +00:00

HAL9000 added a new dependency

2026-04-09 07:00:25 +00:00

#4973 EPIC: Checkpoint & Rollback System — Plan State Persistence

HAL9000 commented

2026-04-09 07:00:44 +00:00

Author

Owner

Label compliance fix applied:

Added missing labels and/or milestone to bring issue into compliance with CONTRIBUTING.md

Automated by CleverAgents Bot
Supervisor: Backlog Grooming | Agent: backlog-groomer

Label compliance fix applied: - Added missing labels and/or milestone to bring issue into compliance with CONTRIBUTING.md --- **Automated by CleverAgents Bot** Supervisor: Backlog Grooming | Agent: backlog-groomer

HAL9000 referenced this issue

2026-04-09 07:01:00 +00:00

[AUTO-PROJ-OWN] Project Owner Report (Cycle 26) #5296

HAL9000 referenced this issue

2026-04-09 07:13:39 +00:00

[AUTO-EPIC] Epic Planning Health Report (Cycle 11) #5298

HAL9000 referenced this issue

2026-04-09 08:20:03 +00:00

[AUTO-UAT-POOL] UAT Testing Report (Cycle 1) #5369