cleveragents/cleveragents-core

Fork 3

bug(cli): plan explain expects decision_id but M3 acceptance test passes plan_id — command fails with exit code 1 #968

New issue

Closed

opened 2026-03-16 03:16:43 +00:00 by freemo · 2 comments

freemo commented

2026-03-16 03:16:43 +00:00

Owner

Metadata

Commit Message: fix(cli): make plan explain accept plan_id for plan-level explanation
Branch: bugfix/m3-plan-explain-plan-id

Background and Context

The M3 (v3.2.0) acceptance test (robot/e2e/m3_acceptance.robot, PR #799) calls plan explain with a plan ID to inspect decision reasoning for a plan:

${r_explain}=    Run CleverAgents Command
...    plan    explain    ${plan_id}    --format    plain

The M3 milestone acceptance criteria state: "agents plan explain shows decision details including alternatives considered." The natural CLI usage pattern is plan explain <plan_id> to see all decisions for a plan.

Current Behavior

The explain_decision_cmd function in src/cleveragents/cli/commands/plan.py (line ~2800) declares its first positional argument as decision_id (a Decision ULID):

def explain_decision_cmd(
    decision_id: Annotated[str, typer.Argument(help="Decision ULID to explain")],
    ...

When the M3 test passes a plan ID, svc.get_decision(plan_id) returns None because the plan ID is not a decision ID. The command then prints "Error: Decision '<plan_id>' not found." and raises typer.Exit(1).

Since Run CleverAgents Command in the test framework defaults to expected_rc=0, the test fails at step 9 (plan explain).

Expected Behavior

plan explain <plan_id> looks up decisions for that plan via decision_service.list_decisions(plan_id)
If decisions are found, the command explains the root decision (or provides a summary of all decisions)
If no decisions are found for the plan, the command falls back to treating the argument as a decision_id for backward compatibility
The command outputs decision details in the requested format without error

Acceptance Criteria

plan explain <plan_id> succeeds when given a plan ID, displaying decisions for that plan
plan explain <decision_id> continues to work for backward compatibility
The command tries the argument as a decision_id first, then falls back to treating it as a plan_id
When treating as plan_id, the root/first decision is explained with full details
The M3 E2E acceptance test step 9 (plan explain) passes without Traceback and with rc=0
Tests (Behave): Existing plan explain scenarios continue to pass
Tests (Robot): M3 acceptance test plan explain step passes
Verify coverage >=97% via nox -s coverage_report
Run nox (all default sessions), fix any errors

Subtasks

Modify explain_decision_cmd to try the identifier as a decision_id first
If not found as decision, look up decisions for the plan via decision_service.list_decisions(identifier)
If decisions exist for the plan, explain the root decision
If neither lookup succeeds, display a clear error message
Tests (Behave): Verify existing plan explain scenarios pass
Tests (Robot): Verify M3 acceptance test step 9 passes
Verify coverage >=97% via nox -s coverage_report
Run nox (all default sessions), fix any errors

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation.
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly.
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.

## Metadata - **Commit Message**: `fix(cli): make plan explain accept plan_id for plan-level explanation` - **Branch**: `bugfix/m3-plan-explain-plan-id` ## Background and Context The M3 (v3.2.0) acceptance test (`robot/e2e/m3_acceptance.robot`, PR #799) calls `plan explain` with a plan ID to inspect decision reasoning for a plan: ```robot ${r_explain}= Run CleverAgents Command ... plan explain ${plan_id} --format plain ``` The M3 milestone acceptance criteria state: *"`agents plan explain` shows decision details including alternatives considered."* The natural CLI usage pattern is `plan explain <plan_id>` to see all decisions for a plan. ## Current Behavior The `explain_decision_cmd` function in `src/cleveragents/cli/commands/plan.py` (line ~2800) declares its first positional argument as `decision_id` (a Decision ULID): ```python def explain_decision_cmd( decision_id: Annotated[str, typer.Argument(help="Decision ULID to explain")], ... ``` When the M3 test passes a plan ID, `svc.get_decision(plan_id)` returns `None` because the plan ID is not a decision ID. The command then prints `"Error: Decision '<plan_id>' not found."` and raises `typer.Exit(1)`. Since `Run CleverAgents Command` in the test framework defaults to `expected_rc=0`, the test fails at step 9 (plan explain). ## Expected Behavior 1. `plan explain <plan_id>` looks up decisions for that plan via `decision_service.list_decisions(plan_id)` 2. If decisions are found, the command explains the root decision (or provides a summary of all decisions) 3. If no decisions are found for the plan, the command falls back to treating the argument as a decision_id for backward compatibility 4. The command outputs decision details in the requested format without error ## Acceptance Criteria - [ ] `plan explain <plan_id>` succeeds when given a plan ID, displaying decisions for that plan - [ ] `plan explain <decision_id>` continues to work for backward compatibility - [ ] The command tries the argument as a decision_id first, then falls back to treating it as a plan_id - [ ] When treating as plan_id, the root/first decision is explained with full details - [ ] The M3 E2E acceptance test step 9 (`plan explain`) passes without Traceback and with rc=0 - [ ] Tests (Behave): Existing plan explain scenarios continue to pass - [ ] Tests (Robot): M3 acceptance test `plan explain` step passes - [ ] Verify coverage >=97% via `nox -s coverage_report` - [ ] Run `nox` (all default sessions), fix any errors ## Subtasks - [ ] Modify `explain_decision_cmd` to try the identifier as a decision_id first - [ ] If not found as decision, look up decisions for the plan via `decision_service.list_decisions(identifier)` - [ ] If decisions exist for the plan, explain the root decision - [ ] If neither lookup succeeds, display a clear error message - [ ] Tests (Behave): Verify existing plan explain scenarios pass - [ ] Tests (Robot): Verify M3 acceptance test step 9 passes - [ ] Verify coverage >=97% via `nox -s coverage_report` - [ ] Run `nox` (all default sessions), fix any errors ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly. - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done.

freemo added the

labels

2026-03-16 03:17:17 +00:00

freemo added this to the v3.2.0 milestone

2026-03-16 03:17:19 +00:00

freemo referenced this issue

2026-03-16 15:59:54 +00:00

TDD: Write failing test for #968 — plan explain expects decision_id but test passes plan_id #978

freemo commented

2026-03-16 16:03:59 +00:00

Author

Owner

TDD workflow initiated for this bug:

Created TDD issue #978 to write a tagged test that captures this bug.
Dependency link needed: this issue is blocked by #978 (please add manually via the UI: #968 depends on #978).
Once #978 is merged to master, @freemo should create branch bugfix/m3-plan-explain-plan-id from master and implement the fix.
The fix must remove the @tdd_expected_fail tag from the test so it runs normally.

Assignees:

TDD issue #978: @brent.edwards
Bug fix (this issue): @freemo

TDD Workflow

This bug follows the project's mandatory TDD workflow for bug fixes (see CONTRIBUTING.md > Bug Fix Workflow).

**TDD workflow initiated for this bug:** - Created TDD issue #978 to write a tagged test that captures this bug. - Dependency link needed: this issue is blocked by #978 (please add manually via the UI: #968 depends on #978). - Once #978 is merged to `master`, @freemo should create branch `bugfix/m3-plan-explain-plan-id` from `master` and implement the fix. - The fix must remove the `@tdd_expected_fail` tag from the test so it runs normally. **Assignees:** - TDD issue #978: @brent.edwards - Bug fix (this issue): @freemo ### TDD Workflow This bug follows the project's mandatory TDD workflow for bug fixes (see `CONTRIBUTING.md` > Bug Fix Workflow).

freemo referenced this issue

2026-03-16 16:22:06 +00:00

fix(cli): Container.resolve() does not exist — plan tree/explain/correct crash with AttributeError #647

freemo referenced this issue

2026-03-17 18:16:43 +00:00

bug(plan): subplan spawn creates metadata but does not orchestrate real child plan execution #823

freemo added the

Points

label

2026-03-17 18:31:25 +00:00

brent.edwards was assigned by freemo

2026-03-17 19:20:07 +00:00

hurui200320 referenced this issue

2026-03-18 07:44:57 +00:00

TDD: Write failing test for #968 — plan explain expects decision_id but test passes plan_id #978

hurui200320 referenced this issue

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

TDD: Write failing test for #968 — plan explain expects decision_id but test passes plan_id #978

hurui200320 referenced this issue from a commit

2026-03-18 09:20:35 +00:00

test: add TDD bug-capture test for #968 — plan explain plan_id handling

hurui200320 referenced this issue

2026-03-18 09:20:58 +00:00

test: add TDD bug-capture test for #968 — plan explain plan_id handling #1052

hurui200320 referenced this issue from a commit

2026-03-18 11:41:09 +00:00

test: add TDD bug-capture test for #968 — plan explain plan_id handling

hurui200320 referenced this issue from a commit

2026-03-18 12:13:20 +00:00

test: add TDD bug-capture test for #968 — plan explain plan_id handling

brent.edwards added

and removed

labels

2026-03-18 18:58:50 +00:00

brent.edwards referenced this issue from a commit

2026-03-18 19:30:45 +00:00

fix(cli): make plan explain accept plan_id for plan-level explanation

brent.edwards commented

2026-03-18 19:40:18 +00:00

Member

Implementation Notes — #968

Branch: bugfix/m3-plan-explain-plan-id | Commit: e9e40ecd

Design Decisions

Modified explain_decision_cmd to accept both decision IDs and plan IDs:

Decision-first resolution — tries svc.get_decision(identifier) first (wrapped in suppress(DecisionNotFoundError)) for backward compatibility
Plan-ID fallback — if not found as decision, tries svc.list_decisions(identifier) treating it as a plan_id, and explains the first (root) decision
Clear error — if neither resolves: '{identifier}' not found as a decision or plan.
Parameter renamed — decision_id → identifier with updated help text

Test Updates

Updated mock in plan_explain_cli_coverage_steps.py to include svc.list_decisions.return_value = [] for the "not found" scenario
All 46 existing explain scenarios continue to pass

Quality Gates

Lint: PASS | Typecheck: PASS (0 errors) | Unit tests: 46/46 scenarios pass

## Implementation Notes — #968 **Branch:** `bugfix/m3-plan-explain-plan-id` | **Commit:** `e9e40ecd` ### Design Decisions Modified `explain_decision_cmd` to accept both decision IDs and plan IDs: 1. **Decision-first resolution** — tries `svc.get_decision(identifier)` first (wrapped in `suppress(DecisionNotFoundError)`) for backward compatibility 2. **Plan-ID fallback** — if not found as decision, tries `svc.list_decisions(identifier)` treating it as a plan_id, and explains the first (root) decision 3. **Clear error** — if neither resolves: `'{identifier}' not found as a decision or plan.` 4. **Parameter renamed** — `decision_id` → `identifier` with updated help text ### Test Updates - Updated mock in `plan_explain_cli_coverage_steps.py` to include `svc.list_decisions.return_value = []` for the "not found" scenario - All 46 existing explain scenarios continue to pass ### Quality Gates - Lint: PASS | Typecheck: PASS (0 errors) | Unit tests: 46/46 scenarios pass

brent.edwards referenced this issue from a pull request that will close it,

2026-03-18 19:40:49 +00:00

fix(cli): make plan explain accept plan_id for plan-level explanation #1057

brent.edwards added a new dependency

2026-03-18 19:41:39 +00:00

#1057 fix(cli): make plan explain accept plan_id for plan-level explanation

brent.edwards added

and removed