CHANGELOG.md

@@ -2,6 +2,13 @@
 
 ## Unreleased
 
+- Added TDD bug-capture tests for bug #968: ``plan explain`` expects a
+  decision_id but the M3 acceptance test passes a plan_id.  Two Behave BDD
+  scenarios (``@tdd_bug @tdd_bug_968 @tdd_expected_fail``) verify the fixed
+  behaviour — ``plan explain <plan_id>`` succeeds (rc=0) and displays
+  decision details.  Includes Robot Framework integration tests with a
+  helper script exercising the same CLI path via subprocess, and step
+  definitions following established patterns.  (#978)
 - Added TDD bug-capture tests for bug #967 — `plan execute` phase processing.
   Tests exercise the CLI orchestration layer via CliRunner (Behave) and
   replicated CLI logic (Robot) to verify that `plan execute` correctly

features/steps/tdd_plan_explain_plan_id_steps.py

@@ -0,0 +1,181 @@
+"""Step definitions for tdd_plan_explain_plan_id.feature.
+
+TDD bug-capture test for bug #968:  ``plan explain`` expects a decision_id
+as its first positional argument, but the M3 acceptance test passes a plan_id.
+Because ``svc.get_decision(plan_id)`` raises ``DecisionNotFoundError`` (the
+plan ID is not a decision ID), the command exits with rc=1 and "Decision not
+found" error.
+
+These steps use the ``@tdd_expected_fail`` tag so that the assertions — which
+expect the *fixed* behaviour (rc=0 with decision details) — do not fail CI
+while the bug is still unfixed.  Once bug #968 is fixed the tag will be
+removed and the tests will run normally.
+"""
+
+from __future__ import annotations
+
+from unittest.mock import MagicMock, patch
+
+from behave import given, then, when
+from behave.runner import Context
+from typer.testing import CliRunner
+from ulid import ULID
+
+from cleveragents.application.services.decision_service import (
+    DecisionNotFoundError,
+)
+from cleveragents.cli.commands.plan import app as plan_app
+from cleveragents.domain.models.core.decision import (
+    Decision,
+    DecisionType,
+)
+
+runner = CliRunner()
+
+_PATCH_CONTAINER = "cleveragents.application.container.get_container"
+
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+
+def _make_decision(
+    decision_id: str | None = None,
+    plan_id: str | None = None,
+    sequence: int = 0,
+    parent_id: str | None = None,
+    dtype: DecisionType = DecisionType.PROMPT_DEFINITION,
+    question: str = "What should we build?",
+    chosen: str = "A REST API",
+) -> Decision:
+    """Build a minimal Decision with sensible defaults."""
+    did = decision_id or str(ULID())
+    pid = plan_id or str(ULID())
+    kwargs: dict = {
+        "decision_id": did,
+        "plan_id": pid,
+        "sequence_number": sequence,
+        "decision_type": dtype,
+        "question": question,
+        "chosen_option": chosen,
+    }
+    if parent_id is not None:
+        kwargs["parent_decision_id"] = parent_id
+    elif dtype != DecisionType.PROMPT_DEFINITION:
+        kwargs["parent_decision_id"] = str(ULID())
+    return Decision(**kwargs)
+
+
+def _mock_container_with_decision_svc(svc_mock: MagicMock) -> MagicMock:
+    """Create a mock container whose ``decision_service()`` returns *svc_mock*."""
+    container = MagicMock()
+    container.decision_service.return_value = svc_mock
+    return container
+
+
+# ---------------------------------------------------------------------------
+# Given steps
+# ---------------------------------------------------------------------------
+
+
+@given(
+    "tdd968 a mock DecisionService where get_decision raises DecisionNotFoundError for a plan id"
+)
+def step_tdd968_mock_get_decision_none(context: Context) -> None:
+    """Simulate the current buggy behaviour: plan ID is not a decision ID."""
+    context.tdd968_plan_id = str(ULID())
+    svc = MagicMock()
+    # get_decision raises DecisionNotFoundError when called with a plan_id (the bug)
+    svc.get_decision.side_effect = DecisionNotFoundError(context.tdd968_plan_id)
+    context.tdd968_svc = svc
+    context.tdd968_container = _mock_container_with_decision_svc(svc)
+
+
+@given(
+    "tdd968 the same mock DecisionService returns decisions "
+    "via list_decisions for the plan id"
+)
+def step_tdd968_mock_list_decisions(context: Context) -> None:
+    """Set up list_decisions to return real decisions for the plan.
+
+    After the fix, the command should fall back to this lookup when
+    get_decision raises DecisionNotFoundError.
+    """
+    root_id = str(ULID())
+    child_id = str(ULID())
+    pid = context.tdd968_plan_id
+    context.tdd968_root_question = "What should we build?"
+    decisions = [
+        _make_decision(
+            decision_id=root_id,
+            plan_id=pid,
+            sequence=0,
+            question=context.tdd968_root_question,
+            chosen="A REST API",
+        ),
+        _make_decision(
+            decision_id=child_id,
+            plan_id=pid,
+            parent_id=root_id,
+            sequence=1,
+            dtype=DecisionType.STRATEGY_CHOICE,
+            question="Which framework?",
+            chosen="FastAPI",
+        ),
+    ]
+    context.tdd968_svc.list_decisions.return_value = decisions
+
+
+# ---------------------------------------------------------------------------
+# When steps
+# ---------------------------------------------------------------------------
+
+
+@when("tdd968 I invoke plan explain with the plan id")
+def step_tdd968_invoke_explain(context: Context) -> None:
+    """Invoke ``plan explain <plan_id>`` via CliRunner with mocked container."""
+    with patch(_PATCH_CONTAINER, return_value=context.tdd968_container):
+        result = runner.invoke(
+            plan_app,
+            ["explain", context.tdd968_plan_id, "--format", "json"],
+        )
+    context.tdd968_result = result
+
+
+# ---------------------------------------------------------------------------
+# Then steps
+# ---------------------------------------------------------------------------
+
+
+@then("tdd968 the command should exit with return code 0")
+def step_tdd968_rc_zero(context: Context) -> None:
+    """Assert rc=0 — this will FAIL while bug #968 is unfixed (rc=1)."""
+    assert context.tdd968_result.exit_code == 0, (
+        f"Expected exit code 0 but got {context.tdd968_result.exit_code}. "
+        f"Output: {context.tdd968_result.output}"
+    )
+
+
+@then("tdd968 the output should contain decision details")
+def step_tdd968_output_has_details(context: Context) -> None:
+    """Assert the output contains decision fields — fails while bug exists."""
+    output = context.tdd968_result.output
+    # When the bug is fixed, the output should contain decision fields
+    assert "decision_id" in output and "question" in output, (
+        f"Expected decision details in output, got: {output}"
+    )
+    # Verify that the fix called list_decisions with the correct plan_id
+    context.tdd968_svc.list_decisions.assert_called_once_with(
+        context.tdd968_plan_id,
+    )
+
+
+@then("tdd968 the output should contain the root decision question")
+def step_tdd968_output_has_question(context: Context) -> None:
+    """Assert the root decision question appears — fails while bug exists."""
+    output = context.tdd968_result.output
+    assert context.tdd968_root_question in output, (
+        f"Expected root question '{context.tdd968_root_question}' "
+        f"in output, got: {output}"
+    )

features/tdd_plan_explain_plan_id.feature

@@ -0,0 +1,35 @@
+@tdd_expected_fail @tdd_bug @tdd_bug_968 @mock_only
+Feature: TDD Bug #968 — plan explain expects decision_id but test passes plan_id
+  As a developer
+  I want to verify that `plan explain <plan_id>` succeeds when given a plan ID
+  So that the bug is captured and will be caught by a regression test
+
+  # This test was written to capture bug #968:
+  # The `plan explain` CLI command declares its first positional argument as
+  # `decision_id` (a Decision ULID).  When the M3 acceptance test passes a
+  # plan ID, `svc.get_decision(plan_id)` raises DecisionNotFoundError because
+  # the plan ID is not a decision ID.  The command exits with rc=1 and
+  # "Decision not found".
+  #
+  # The expected fix (#968) will make `explain_decision_cmd` fall back to
+  # treating the argument as a plan_id when decision lookup fails — looking
+  # up decisions for the plan via `decision_service.list_decisions(plan_id)`
+  # and explaining the root decision.
+  #
+  # These tests assert the *fixed* behaviour (rc=0 with decision details) and
+  # will FAIL until the bug is fixed.  The @tdd_expected_fail tag inverts the
+  # result so CI passes.
+
+  Scenario: Plan explain succeeds when given a plan_id with decisions
+    Given tdd968 a mock DecisionService where get_decision raises DecisionNotFoundError for a plan id
+    And tdd968 the same mock DecisionService returns decisions via list_decisions for the plan id
+    When tdd968 I invoke plan explain with the plan id
+    Then tdd968 the command should exit with return code 0
+    And tdd968 the output should contain decision details
+
+  Scenario: Plan explain with plan_id shows root decision question
+    Given tdd968 a mock DecisionService where get_decision raises DecisionNotFoundError for a plan id
+    And tdd968 the same mock DecisionService returns decisions via list_decisions for the plan id
+    When tdd968 I invoke plan explain with the plan id
+    Then tdd968 the command should exit with return code 0
+    And tdd968 the output should contain the root decision question

robot/helper_tdd_plan_explain_plan_id.py

@@ -0,0 +1,197 @@
+"""Helper script for tdd_plan_explain_plan_id.robot integration tests.
+
+Each subcommand exercises the ``plan explain <plan_id>`` CLI path to reproduce
+bug #968.  The ``plan explain`` command currently declares its first positional
+argument as ``decision_id``.  When passed a plan ID, ``svc.get_decision(plan_id)``
+raises ``DecisionNotFoundError`` (the plan ID is not a decision ID) and the
+command exits with rc=1 and "Decision not found" error.
+
+The helper exits 0 with a sentinel when the command succeeds (bug fixed), and
+exits 1 when the bug is still present.  The ``tdd_expected_fail_listener`` on
+the Robot side handles pass/fail inversion while the bug remains open.
+"""
+
+from __future__ import annotations
+
+import os
+import subprocess
+import sys
+from collections.abc import Callable
+from pathlib import Path
+from typing import NoReturn
+
+# Ensure local source tree AND robot/ directory are importable.
+_ROOT: Path = Path(__file__).resolve().parents[1]
+_SRC: str = str(_ROOT / "src")
+_ROBOT: str = str(_ROOT / "robot")
+for _p in (_SRC, _ROBOT):
+    if _p not in sys.path:
+        sys.path.insert(0, _p)
+
+from ulid import ULID  # noqa: E402
+
+from cleveragents.application.container import get_container  # noqa: E402
+from cleveragents.domain.models.core.decision import DecisionType  # noqa: E402
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+
+def _fail(msg: str) -> NoReturn:
+    """Print failure message to stderr and exit with code 1."""
+    print(msg, file=sys.stderr)
+    sys.exit(1)
+
+
+def _make_subprocess_env() -> dict[str, str]:
+    """Build environment for subprocess calls with ``NO_COLOR=1``."""
+    env: dict[str, str] = os.environ.copy()
+    env["NO_COLOR"] = "1"
+    return env
+
+
+def _setup_plan_with_decisions() -> str:
+    """Create a plan_id and record decisions against it.
+
+    Uses the DecisionService directly with a synthetic plan ID.
+    The decision service does not require an actual Plan object to exist —
+    it records decisions keyed by plan_id string.
+
+    Returns the plan_id with recorded decisions.
+    """
+    container = get_container()
+    decision_svc = container.decision_service()
+
+    plan_id: str = str(ULID())
+
+    # Record a root decision against this plan
+    decision_svc.record_decision(
+        plan_id=plan_id,
+        decision_type=DecisionType.PROMPT_DEFINITION,
+        question="What should we build?",
+        chosen_option="A REST API",
+        rationale="REST API is the most common pattern",
+    )
+
+    # Defensive check: verify that the decision was persisted before the
+    # subprocess reads it (distinguishes setup failures from the actual bug).
+    decisions = decision_svc.list_decisions(plan_id)
+    if not decisions:
+        _fail(
+            f"Setup failure: record_decision succeeded but list_decisions "
+            f"returned no decisions for plan_id={plan_id}. "
+            f"This is a test setup problem, not bug #968."
+        )
+
+    return plan_id
+
+
+def _run_plan_explain(plan_id: str) -> subprocess.CompletedProcess[str]:
+    """Run ``plan explain <plan_id>`` via subprocess.
+
+    Handles timeout with a descriptive error and sets ``NO_COLOR=1`` to
+    prevent ANSI escape codes in captured output.
+    """
+    try:
+        return subprocess.run(
+            [
+                sys.executable,
+                "-m",
+                "cleveragents",
+                "plan",
+                "explain",
+                plan_id,
+                "--format",
+                "plain",
+            ],
+            capture_output=True,
+            text=True,
+            timeout=45,
+            cwd=str(_ROOT),
+            env=_make_subprocess_env(),
+        )
+    except subprocess.TimeoutExpired:
+        _fail(
+            f"plan explain {plan_id} timed out after 45 seconds. "
+            f"Bug #968: subprocess exceeded inner timeout."
+        )
+
+
+# ---------------------------------------------------------------------------
+# Subcommands
+# ---------------------------------------------------------------------------
+
+
+def explain_with_plan_id() -> None:
+    """Verify that ``plan explain <plan_id>`` succeeds (rc=0).
+
+    Bug #968: The command currently treats the argument as a decision_id,
+    calls ``svc.get_decision(plan_id)`` which raises DecisionNotFoundError,
+    and exits with rc=1.  When the fix is applied, the command should fall
+    back to looking up decisions for the plan via ``list_decisions(plan_id)``
+    and explain the root decision.
+    """
+    plan_id: str = _setup_plan_with_decisions()
+    result: subprocess.CompletedProcess[str] = _run_plan_explain(plan_id)
+
+    if result.returncode != 0:
+        _fail(
+            f"plan explain {plan_id} exited with rc={result.returncode}. "
+            f"stdout: {result.stdout}\n"
+            f"stderr: {result.stderr}\n"
+            f"Bug #968: explain treats the argument as a decision_id, "
+            f"get_decision(plan_id) raises DecisionNotFoundError, command fails."
+        )
+
+    # Verify the output contains decision-related content — both keywords
+    # must be present (mirrors the AND-based assertion in the Behave test).
+    combined: str = result.stdout + result.stderr
+    if "decision" not in combined.lower() or "question" not in combined.lower():
+        _fail(
+            f"plan explain output does not contain decision details. "
+            f"stdout: {result.stdout}"
+        )
+
+    print("tdd-plan-explain-plan-id-ok")
+
+
+def explain_plan_id_shows_question() -> None:
+    """Verify that ``plan explain <plan_id>`` shows the root decision question.
+
+    Bug #968: Since the command fails with rc=1 before any output is
+    rendered, the root decision question is never displayed.
+    """
+    plan_id: str = _setup_plan_with_decisions()
+    result: subprocess.CompletedProcess[str] = _run_plan_explain(plan_id)
+
+    if result.returncode != 0:
+        _fail(
+            f"plan explain {plan_id} exited with rc={result.returncode}. "
+            f"Bug #968: command fails before rendering any output."
+        )
+
+    if "What should we build?" not in result.stdout:
+        _fail(f"Expected root decision question in output. stdout: {result.stdout}")
+
+    print("tdd-plan-explain-plan-id-question-ok")
+
+
+# ---------------------------------------------------------------------------
+# Dispatcher
+# ---------------------------------------------------------------------------
+
+_COMMANDS: dict[str, Callable[[], None]] = {
+    "explain-with-plan-id": explain_with_plan_id,
+    "explain-plan-id-shows-question": explain_plan_id_shows_question,
+}
+
+if __name__ == "__main__":
+    if len(sys.argv) < 2 or sys.argv[1] not in _COMMANDS:
+        print(
+            f"Usage: {sys.argv[0]} <{'|'.join(_COMMANDS)}>",
+            file=sys.stderr,
+        )
+        sys.exit(1)
+    cmd: Callable[[], None] = _COMMANDS[sys.argv[1]]
+    cmd()

robot/tdd_plan_explain_plan_id.robot

@@ -0,0 +1,36 @@
+*** Settings ***
+Documentation    TDD Bug #968 — plan explain expects decision_id but M3 test passes plan_id
+...              Integration tests verifying that ``plan explain <plan_id>`` succeeds
+...              when given a plan ID rather than a decision ID.  Currently the command
+...              treats the argument as a decision_id, calls ``svc.get_decision(plan_id)``
+...              which returns None, and exits with rc=1 and "Decision not found" error.
+...              Tests are tagged tdd_expected_fail so CI passes via result inversion.
+Resource         ${CURDIR}/common.resource
+Suite Setup      Setup Test Environment With Database Isolation
+Suite Teardown   Cleanup Test Environment
+
+*** Variables ***
+${HELPER}    ${CURDIR}/helper_tdd_plan_explain_plan_id.py
+
+*** Test Cases ***
+TDD Plan Explain Succeeds With Plan ID
+    [Documentation]    Verify that ``plan explain <plan_id>`` exits with rc=0
+    ...                when given a plan ID that has associated decisions.
+    ...                Bug #968: the command currently exits with rc=1.
+    [Tags]    tdd_expected_fail    tdd_bug    tdd_bug_968
+    ${result}=    Run Process    ${PYTHON}    ${HELPER}    explain-with-plan-id    cwd=${WORKSPACE}    timeout=60s    on_timeout=kill
+    Log    ${result.stdout}
+    Log    ${result.stderr}
+    Should Be Equal As Integers    ${result.rc}    0
+    Should Contain    ${result.stdout}    tdd-plan-explain-plan-id-ok
+
+TDD Plan Explain With Plan ID Shows Root Question
+    [Documentation]    Verify that ``plan explain <plan_id>`` output contains
+    ...                the root decision question when given a plan ID.
+    ...                Bug #968: the command fails before rendering any output.
+    [Tags]    tdd_expected_fail    tdd_bug    tdd_bug_968
+    ${result}=    Run Process    ${PYTHON}    ${HELPER}    explain-plan-id-shows-question    cwd=${WORKSPACE}    timeout=60s    on_timeout=kill
+    Log    ${result.stdout}
+    Log    ${result.stderr}
+    Should Be Equal As Integers    ${result.rc}    0
+    Should Contain    ${result.stdout}    tdd-plan-explain-plan-id-question-ok