features/mocks/tdd_plan_correct_plan_id_fixtures.py

@@ -16,9 +16,20 @@ TDD: https://git.cleverthis.com/cleveragents/cleveragents-core/issues/979
 
 from __future__ import annotations
 
+from datetime import datetime
 from types import SimpleNamespace
 from unittest.mock import MagicMock
 
+from cleveragents.domain.models.core.plan import (
+    NamespacedName,
+    Plan,
+    PlanIdentity,
+    PlanPhase,
+    PlanTimestamps,
+    ProcessingState,
+    ProjectLink,
+)
+
 # ---------------------------------------------------------------------------
 # Patch targets
 # ---------------------------------------------------------------------------
@@ -58,16 +69,47 @@ def make_decision_ns(
     )
 
 
+def _make_plan(_plan_id: str) -> Plan:
+    """Build a real ``Plan`` domain object so ``isinstance`` checks pass.
+
+    The *_plan_id* argument is accepted for API symmetry but not used
+    directly because ``PlanIdentity.plan_id`` must be a valid 26-char
+    ULID.  A fresh ULID is generated instead.
+    """
+    from ulid import ULID
+
+    return Plan(
+        identity=PlanIdentity(plan_id=str(ULID())),
+        namespaced_name=NamespacedName(namespace="local", name="tdd-969-plan"),
+        action_name="local/tdd-969-action",
+        description="TDD plan for bug #969",
+        phase=PlanPhase.EXECUTE,
+        processing_state=ProcessingState.COMPLETE,
+        project_links=[ProjectLink(project_name="proj-1")],
+        strategy_actor="openai/gpt-4",
+        execution_actor="openai/gpt-4",
+        timestamps=PlanTimestamps(created_at=datetime.now(), updated_at=datetime.now()),
+    )
+
+
 def make_mock_container(
     decisions: list[SimpleNamespace],
     influence_edges: dict[str, list[str]],
 ) -> MagicMock:
-    """Build a mock DI container returning a DecisionService."""
+    """Build a mock DI container returning DecisionService and PlanLifecycleService."""
     mock_decision_svc = MagicMock()
     mock_decision_svc.list_decisions.return_value = decisions
     mock_decision_svc.get_influence_edges.return_value = influence_edges
+
+    # The fix for bug #969 calls plan_lifecycle_service().get_plan()
+    # to detect whether the identifier is a plan_id.  We must return
+    # a real Plan so that ``isinstance(plan_obj, Plan)`` passes.
+    mock_plan_svc = MagicMock()
+    mock_plan_svc.get_plan.return_value = _make_plan(PLAN_ID)
+
     mock_container = MagicMock()
     mock_container.decision_service.return_value = mock_decision_svc
+    mock_container.plan_lifecycle_service.return_value = mock_plan_svc
     return mock_container
 
 

src/cleveragents/cli/commands/plan.py

@@ -2671,9 +2671,11 @@ def plan_artifacts(
 
 @app.command("correct")
 def correct_decision(
-    decision_id: Annotated[
+    identifier: Annotated[
         str,
-        typer.Argument(help="Decision ID to correct"),
+        typer.Argument(
+            help="Plan ID (auto-selects root decision) or Decision ID to correct"
+        ),
     ],
     mode: Annotated[
         str,
@@ -2727,6 +2729,10 @@ def correct_decision(
 ) -> None:
     """Correct a decision in a plan's decision tree.
 
+    The positional *identifier* can be either a **plan ID** or a
+    **decision ID**.  When a plan ID is given the root decision of that
+    plan is automatically selected as the correction target.
+
     Supports two modes:
 
     * **revert** -- undo a decision and recompute affected subtrees
@@ -2736,6 +2742,7 @@ def correct_decision(
 
     Examples::
 
+        agents plan correct --mode revert -g "Use FastAPI instead" PLAN-001
         agents plan correct --mode revert -g "Use FastAPI instead" DEC-001
         agents plan correct --mode append -g "Add caching layer" --dry-run DEC-002
     """
@@ -2760,29 +2767,44 @@ def correct_decision(
             console.print("[red]Error:[/red] --guidance / -g must not be blank.")
             raise typer.Abort()
 
-        # Resolve plan_id
-        resolved_plan_id = plan_id or _resolve_active_plan_id()
-
-        # Resolve DecisionService via DI to build the structural tree
-        # and influence DAG for affected-subtree computation (issue #606).
+        # Resolve identifier: try as plan_id first, then fall back to
+        # decision_id for backward compatibility (issue #969).
         from cleveragents.application.container import get_container
+        from cleveragents.domain.models.core.plan import Plan
 
         container = get_container()
         decision_svc = container.decision_service()
 
-        # Build structural tree adjacency list (parent -> children)
-        decisions = decision_svc.list_decisions(resolved_plan_id)
+        target_decision_id: str
+        resolved_plan_id: str
+        _is_plan = False
+        try:
+            service = container.plan_lifecycle_service()
+            plan_obj = service.get_plan(identifier)
+            if isinstance(plan_obj, Plan):
+                _is_plan = True
+        except RNF:
+            # Lookup failed - identifier is not a plan_id
+            pass
 
-        # Bug #969: detect if decision_id is actually a plan_id
-        known_decision_ids = {d.decision_id for d in decisions}
-        target_decision_id = decision_id
-        if target_decision_id not in known_decision_ids:
-            # Treat it as a plan_id: re-resolve plan and find root decision
-            resolved_plan_id = target_decision_id
+        if _is_plan:
+            # identifier is a valid plan_id - auto-select root decision
+            resolved_plan_id = identifier
             decisions = decision_svc.list_decisions(resolved_plan_id)
             root_decisions = [d for d in decisions if d.parent_decision_id is None]
-            if root_decisions:
-                target_decision_id = root_decisions[0].decision_id
+            if not root_decisions:
+                console.print(
+                    f"[red]Error:[/red] Plan '{identifier}' has no root decision."
+                )
+                raise typer.Abort()
+            target_decision_id = root_decisions[0].decision_id
+        else:
+            # Not a plan_id - treat as decision_id (backward compat)
+            target_decision_id = identifier
+            resolved_plan_id = plan_id or _resolve_active_plan_id()
+
+        # Build structural tree adjacency list (parent -> children)
+        decisions = decision_svc.list_decisions(resolved_plan_id)
 
         decision_tree: dict[str, list[str]] = {}
         for d in decisions:
@@ -2846,7 +2868,7 @@ def correct_decision(
         if not yes:
             console.print(
                 f"\n[bold]Correction:[/bold] {correction_mode.value} "
-                f"decision {decision_id}"
+                f"decision {target_decision_id}"
             )
             console.print(f"[bold]Guidance:[/bold] {guidance}")
             confirm = typer.confirm("\nProceed with correction?")