fix(plan-correct): wrap JSON output in spec-required envelope with data.correction

The  CLI command was outputting a flat dict
with mode and correction_id at the top level under data, instead of
nesting them under data.correction as required by the specification.

This fix:
- Builds an envelope with command='plan correct' for JSON/YAML output
- Wraps correction fields (correction_id, mode, status, etc.) under
  a nested 'data.correction' key so that data.correction.mode is present
- Applies the same envelope structure to both dry-run and execute paths

Fixes #8584

CHANGELOG.md

@@ -421,13 +421,19 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
   constructing `LLMStrategizeActor`.  `run_strategize` in `PlanExecutor` now
   passes `resources` (derived from `plan.project_links`) and `project_context`
   to the actor so the LLM prompt receives full project context.  Strategy
-  decisions are serialised as JSON in `plan.error_details["strategy_decisions_json"]`
+  decisions are serialised as JSON in `plan.error_details["strategy_decisions_json"]` 
   so `_build_decisions` can reconstruct the full hierarchy (dependency ordering,
   parent/child structure) during Execute instead of rebuilding from
   `definition_of_done`.  `StrategizeStubActor.execute` accepts `**kwargs` for
   forward-compatibility.  Added BDD coverage for the stored-JSON path,
   corrupt-JSON fallback, resource-passing, and stub extra-kwargs scenarios. (#828)
 
+- **TDD Issue-Capture Test: plan correct JSON envelope** (#8584): Added a
+  failing BDD scenario tagged ``@tdd_expected_fail`` that proves the
+  ``plan correct --format json`` command outputs a flat dict instead of the
+  spec-required CLI envelope (``command``, ``status``, ``exit_code``, ``data``,
+  ``timing``, ``messages``).  The scenario also asserts ``data.correction.mode``
+  is present.  CI treats the failure as expected until the fix is merged.
 
 - **TDD Issue-Capture Test Activation** (#7025): Replaced 234 bare `@skip` tags
   across 82 Behave feature files with the correct `@tdd_expected_fail @tdd_issue

features/mocks/tdd_plan_correct_json_output_fixtures.py

@@ -0,0 +1,159 @@
+"""Shared mock fixtures for TDD plan-correct JSON output envelope tests.
+
+Provides constants, mock builders, and CLI argument helpers used by the
+Behave step definitions
+(``features/steps/tdd_plan_correct_json_output_steps.py``).
+
+Centralising the mock builders ensures the test suite exercises the
+``plan correct`` CLI JSON output path with identically-shaped mock objects.
+
+Bug: https://git.cleverthis.com/cleveragents/cleveragents-core/issues/8584
+"""
+
+from __future__ import annotations
+
+from datetime import datetime
+from types import SimpleNamespace
+from unittest.mock import MagicMock
+
+from cleveragents.core.exceptions import ResourceNotFoundError
+from cleveragents.domain.models.core.plan import (
+    NamespacedName,
+    Plan,
+    PlanIdentity,
+    PlanPhase,
+    PlanTimestamps,
+    ProcessingState,
+    ProjectLink,
+)
+
+# ---------------------------------------------------------------------------
+# Patch targets
+# ---------------------------------------------------------------------------
+
+PATCH_CONTAINER: str = "cleveragents.application.container.get_container"
+
+# ---------------------------------------------------------------------------
+# Fixed identifiers for deterministic assertions
+# ---------------------------------------------------------------------------
+
+DECISION_ID: str = "DEC-8584-TARGET"
+ROOT_DECISION_ID: str = "DEC-8584-ROOT"
+CORRECTION_ID: str = "CORR-TDD-8584"
+
+
+# ---------------------------------------------------------------------------
+# Mock builders
+# ---------------------------------------------------------------------------
+
+
+def make_decision_ns(
+    decision_id: str,
+    parent_decision_id: str | None,
+) -> SimpleNamespace:
+    """Create a minimal decision-like namespace for list_decisions."""
+    return SimpleNamespace(
+        decision_id=decision_id,
+        parent_decision_id=parent_decision_id,
+    )
+
+
+def _make_plan() -> Plan:
+    """Build a real ``Plan`` in Execute/COMPLETE state."""
+    from ulid import ULID
+
+    return Plan(
+        identity=PlanIdentity(plan_id=str(ULID())),
+        namespaced_name=NamespacedName(
+            server=None, namespace="local", name="tdd-8584-plan"
+        ),
+        action_name="local/tdd-8584-action",
+        description="TDD plan for bug #8584 — JSON output envelope",
+        definition_of_done=None,
+        phase=PlanPhase.EXECUTE,
+        processing_state=ProcessingState.COMPLETE,
+        project_links=[ProjectLink(project_name="proj-1")],
+        strategy_actor="openai/gpt-4",
+        execution_actor="openai/gpt-4",
+        created_by=None,
+        reusable=False,
+        read_only=False,
+        timestamps=PlanTimestamps(created_at=datetime.now(), updated_at=datetime.now()),
+    )
+
+
+def make_container(mode: str = "revert") -> MagicMock:
+    """Build a mock DI container for the JSON output envelope test.
+
+    Args:
+        mode: The correction mode (``"revert"`` or ``"append"``).
+    """
+    plan = _make_plan()
+
+    mock_plan_svc = MagicMock()
+    # get_plan raises RNF for the decision_id (it is not a plan_id),
+    # ensuring the code falls through to the decision_id path.
+    mock_plan_svc.get_plan.side_effect = ResourceNotFoundError(
+        resource_type="Plan",
+        resource_id=DECISION_ID,
+    )
+    mock_plan_svc.list_plans.return_value = [plan]
+
+    decisions = [
+        make_decision_ns(ROOT_DECISION_ID, None),
+        make_decision_ns(DECISION_ID, ROOT_DECISION_ID),
+    ]
+
+    mock_decision_svc = MagicMock()
+    mock_decision_svc.list_decisions.return_value = decisions
+    mock_decision_svc.get_influence_edges.return_value = {}
+
+    mock_correction_svc = MagicMock()
+    mock_correction_svc.request_correction.return_value = SimpleNamespace(
+        correction_id=CORRECTION_ID,
+        mode=SimpleNamespace(value=mode),
+        target_decision_id=DECISION_ID,
+        guidance="Recompute decision subtree",
+    )
+    mock_correction_svc.execute_correction.return_value = SimpleNamespace(
+        correction_id=CORRECTION_ID,
+        status=SimpleNamespace(value="applied"),
+        reverted_decisions=[DECISION_ID],
+        new_decisions=[],
+    )
+
+    mock_container = MagicMock()
+    mock_container.plan_lifecycle_service.return_value = mock_plan_svc
+    mock_container.decision_service.return_value = mock_decision_svc
+    mock_container.correction_service.return_value = mock_correction_svc
+    return mock_container
+
+
+def build_cli_args(mode: str = "revert") -> list[str]:
+    """Build the CLI argument list for ``plan correct <decision_id> --format json``.
+
+    Args:
+        mode: The correction mode (``"revert"`` or ``"append"``).
+    """
+    return [
+        "correct",
+        DECISION_ID,
+        "--mode",
+        mode,
+        "--guidance",
+        "Recompute decision subtree",
+        "--yes",
+        "--format",
+        "json",
+    ]
+
+
+__all__: list[str] = [
+    "CORRECTION_ID",
+    "DECISION_ID",
+    "PATCH_CONTAINER",
+    "ROOT_DECISION_ID",
+    "build_cli_args",
+    "make_container",
+    "make_decision_ns",
+]

features/steps/tdd_plan_correct_json_output_steps.py

@@ -0,0 +1,192 @@
+"""Step definitions for tdd_plan_correct_json_output.feature.
+
+Captures bug #8584: the ``plan correct`` CLI command's JSON output
+(``--format json``) does not match the spec-required envelope structure.
+
+The current implementation places correction fields directly under ``data``
+(``data.mode``, ``data.correction_id``) instead of nesting them under
+``data.correction``.  This means ``data.correction.mode`` is absent from
+the output.  Additionally, the ``command`` field is empty instead of
+``"plan correct"``.
+
+The assertions verify the *expected* (correct) behaviour.  They will
+**fail** on the current codebase, proving the bug exists.  The
+``@tdd_expected_fail`` tag inverts the result so CI passes.
+
+All step text uses the ``tpcjo`` prefix to avoid collisions with other
+step files.
+
+Bug: https://git.cleverthis.com/cleveragents/cleveragents-core/issues/8584
+"""
+
+from __future__ import annotations
+
+import json
+from unittest.mock import patch
+
+from behave import given, then, when
+from behave.runner import Context
+from typer.testing import CliRunner
+
+from features.mocks.tdd_plan_correct_json_output_fixtures import (
+    PATCH_CONTAINER,
+    build_cli_args,
+    make_container,
+)
+
+runner = CliRunner()
+
+
+def _get_plan_app():
+    """Lazily import the plan CLI app to avoid slow module-level import.
+
+    ``cleveragents.cli.commands.plan`` triggers a large import chain
+    (application container, all services, etc.) that takes ~100 s on a
+    cold interpreter.  Deferring the import to the first step execution
+    means the module is already cached by the time these scenarios run,
+    so the actual cost is near-zero.
+    """
+    from cleveragents.cli.commands.plan import app as plan_app
+
+    return plan_app
+
+
+# ---------------------------------------------------------------------------
+# GIVEN steps
+# ---------------------------------------------------------------------------
+
+
+@given(
+    "tpcjo a container with a plan and a correction service"
+    " that succeeds in revert mode"
+)
+def step_tpcjo_container_revert(context: Context) -> None:
+    """Set up a mock DI container for revert mode."""
+    context.tpcjo_container = make_container(mode="revert")
+    context.tpcjo_mode = "revert"
+
+
+@given(
+    "tpcjo a container with a plan and a correction service"
+    " that succeeds in append mode"
+)
+def step_tpcjo_container_append(context: Context) -> None:
+    """Set up a mock DI container for append mode."""
+    context.tpcjo_container = make_container(mode="append")
+    context.tpcjo_mode = "append"
+
+
+# ---------------------------------------------------------------------------
+# WHEN steps
+# ---------------------------------------------------------------------------
+
+
+@when("tpcjo I invoke plan correct with --format json in revert mode")
+def step_tpcjo_invoke_revert(context: Context) -> None:
+    """Invoke ``plan correct <decision_id> --format json --mode revert``.
+
+    Patches ``get_container`` so the command uses the mock DI container
+    instead of the real application container.
+    """
+    args = build_cli_args(mode="revert")
+    with patch(PATCH_CONTAINER, return_value=context.tpcjo_container):
+        context.tpcjo_result = runner.invoke(_get_plan_app(), args)
+    _parse_json_output(context)
+
+
+@when("tpcjo I invoke plan correct with --format json in append mode")
+def step_tpcjo_invoke_append(context: Context) -> None:
+    """Invoke ``plan correct <decision_id> --format json --mode append``.
+
+    Patches ``get_container`` so the command uses the mock DI container
+    instead of the real application container.
+    """
+    args = build_cli_args(mode="append")
+    with patch(PATCH_CONTAINER, return_value=context.tpcjo_container):
+        context.tpcjo_result = runner.invoke(_get_plan_app(), args)
+    _parse_json_output(context)
+
+
+# ---------------------------------------------------------------------------
+# THEN steps
+# ---------------------------------------------------------------------------
+
+
+@then('tpcjo the JSON output data.correction.mode should be "{expected_mode}"')
+def step_tpcjo_json_correction_mode(context: Context, expected_mode: str) -> None:
+    """Assert the parsed JSON output has data.correction.mode set correctly.
+
+    This assertion will FAIL on the current codebase because the
+    implementation places ``mode`` directly under ``data`` (as ``data.mode``)
+    instead of nesting it under ``data.correction`` (as ``data.correction.mode``).
+    """
+    parsed = context.tpcjo_parsed_json
+    assert "data" in parsed, (
+        f"Bug #8584: JSON output is missing top-level 'data' key.  "
+        f"Current output keys: {list(parsed.keys())}.  "
+        f"Full output: {context.tpcjo_result.output!r}"
+    )
+    data = parsed["data"]
+    assert isinstance(data, dict), (
+        f"Bug #8584: JSON output 'data' value is not a dict: {data!r}"
+    )
+    assert "correction" in data, (
+        f"Bug #8584: JSON output 'data' is missing 'correction' key.  "
+        f"Current data keys: {list(data.keys())}.  "
+        f"The spec requires correction fields to be nested under data.correction, "
+        f"but the current implementation places them directly under data "
+        f"(data.mode, data.correction_id, etc.)."
+    )
+    correction = data["correction"]
+    assert isinstance(correction, dict), (
+        f"Bug #8584: JSON output 'data.correction' is not a dict: {correction!r}"
+    )
+    assert "mode" in correction, (
+        f"Bug #8584: JSON output 'data.correction' is missing 'mode' key.  "
+        f"Current correction keys: {list(correction.keys())}"
+    )
+    actual_mode = correction["mode"]
+    assert actual_mode == expected_mode, (
+        f"Bug #8584: data.correction.mode is {actual_mode!r}, "
+        f"expected {expected_mode!r}"
+    )
+
+
+@then('tpcjo the JSON output command field should be "{expected_command}"')
+def step_tpcjo_json_command_field(context: Context, expected_command: str) -> None:
+    """Assert the parsed JSON output has the correct command field value.
+
+    This assertion will FAIL on the current codebase because the
+    ``command`` field is an empty string instead of ``"plan correct"``.
+    """
+    parsed = context.tpcjo_parsed_json
+    assert "command" in parsed, (
+        f"Bug #8584: JSON output is missing top-level 'command' key.  "
+        f"Current output keys: {list(parsed.keys())}.  "
+        f"Full output: {context.tpcjo_result.output!r}"
+    )
+    actual_command = parsed["command"]
+    assert actual_command == expected_command, (
+        f"Bug #8584: command field is {actual_command!r}, "
+        f"expected {expected_command!r}.  "
+        f"The spec requires command to be 'plan correct' but the current "
+        f"implementation sets it to an empty string."
+    )
+
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+
+def _parse_json_output(context: Context) -> None:
+    """Parse the CLI output as JSON and store it on the context.
+
+    If the command failed or the output is not valid JSON, stores an
+    empty dict so that subsequent assertions produce clear failure messages.
+    """
+    result = context.tpcjo_result
+    try:
+        context.tpcjo_parsed_json = json.loads(result.output)
+    except (json.JSONDecodeError, ValueError):
+        context.tpcjo_parsed_json = {}

features/tdd_plan_correct_json_output.feature

@@ -0,0 +1,56 @@
+@tdd_issue @tdd_issue_8584
+Feature: TDD Bug #8584 - plan correct JSON output missing spec-required envelope format
+  As a developer
+  I want plan correct --format json to return the standard CLI envelope
+  So that JSON consumers receive the spec-required nested data structure
+
+  The v3.2.0 specification (section CLI Commands - agents plan correct, line 14912 in
+  docs/specification.md) defines the required JSON output envelope for
+  agents plan correct --format json.
+
+  The spec requires the correction data to be nested under data.correction:
+
+      {
+        "command": "plan correct",
+        "status": "ok",
+        "exit_code": 0,
+        "data": {
+          "correction": {
+            "mode": "revert",
+            "impact": "...",
+            ...
+          },
+          "affected_subtree": {...},
+          ...
+        },
+        "timing": {...},
+        "messages": ["Correction applied"]
+      }
+
+  The current implementation places correction fields directly under data
+  (data.mode, data.correction_id) instead of nesting them under data.correction.
+  This means data.correction.mode is absent from the output.
+
+  These scenarios assert the expected (correct) behaviour and will FAIL
+  against the current implementation, proving the gap exists.
+  The @tdd_expected_fail tag inverts the result so CI passes.
+
+  All step text uses the tpcjo prefix to avoid collisions with other step files.
+
+  @tdd_issue @tdd_issue_8584 @tdd_expected_fail
+  Scenario: plan correct --format json data.correction.mode is present in revert mode
+    Given tpcjo a container with a plan and a correction service that succeeds in revert mode
+    When tpcjo I invoke plan correct with --format json in revert mode
+    Then tpcjo the JSON output data.correction.mode should be "revert"
+
+  @tdd_issue @tdd_issue_8584 @tdd_expected_fail
+  Scenario: plan correct --format json data.correction.mode is present in append mode
+    Given tpcjo a container with a plan and a correction service that succeeds in append mode
+    When tpcjo I invoke plan correct with --format json in append mode
+    Then tpcjo the JSON output data.correction.mode should be "append"
+
+  @tdd_issue @tdd_issue_8584 @tdd_expected_fail
+  Scenario: plan correct --format json command field is set to plan correct
+    Given tpcjo a container with a plan and a correction service that succeeds in revert mode
+    When tpcjo I invoke plan correct with --format json in revert mode
+    Then tpcjo the JSON output command field should be "plan correct"

src/cleveragents/cli/commands/plan.py

@@ -3363,15 +3363,19 @@ def correct_decision(
             )
             if fmt != OutputFormat.RICH.value:
                 data = {
-                    "correction_id": request.correction_id,
-                    "mode": request.mode.value,
-                    "target_decision": request.target_decision_id,
-                    "affected_decisions": impact.affected_decisions,
-                    "affected_files": impact.affected_files,
-                    "estimated_cost": impact.estimated_cost,
-                    "risk_level": impact.risk_level,
+                    "correction": {
+                        "correction_id": request.correction_id,
+                        "mode": request.mode.value,
+                        "target_decision": request.target_decision_id,
+                        "affected_decisions": impact.affected_decisions,
+                        "affected_files": impact.affected_files,
+                        "estimated_cost": impact.estimated_cost,
+                        "risk_level": impact.risk_level,
+                    },
                 }
-                console.print(format_output(data, fmt))
+                console.print(
+                    format_output(data, fmt, command="plan correct"),
+                )
             else:
                 console.print(
                     Panel(
@@ -3414,13 +3418,17 @@ def correct_decision(
 
         if fmt != OutputFormat.RICH.value:
             data = {
-                "correction_id": result.correction_id,
-                "status": result.status.value,
-                "mode": correction_mode.value,
-                "new_decisions": result.new_decisions,
-                "reverted_decisions": result.reverted_decisions,
+                "correction": {
+                    "correction_id": result.correction_id,
+                    "mode": correction_mode.value,
+                    "status": result.status.value,
+                    "new_decisions": result.new_decisions,
+                    "reverted_decisions": result.reverted_decisions,
+                },
             }
-            console.print(format_output(data, fmt))
+            console.print(
+                format_output(data, fmt, command="plan correct"),
+            )
         else:
             console.print(
                 f"[green]✓[/green] Correction applied: {result.correction_id}"