From a02cd87448fbba4400736716aa958a5685b90f18 Mon Sep 17 00:00:00 2001 From: CleverThis Date: Wed, 6 May 2026 06:02:33 +0000 Subject: [PATCH] fix(cli): wrap plan apply --format json output in spec-required JSON envelope (#9817) The `agents plan apply --format json` command now produces a properly structured JSON envelope with all required fields (command, status, exit_code, data, timing, messages). Previously the output used raw plan data without the spec-required envelope wrapper, making it inconsistent with `plan execute --format json`. Changes: - Add `_apply_output_dict()` function to build the spec-required JSON envelope for plan apply (matching `_execute_output_dict` pattern) - Update `lifecycle_apply_plan()` to use the new envelope instead of raw data - Add BDD scenarios verifying the envelope structure for apply output - Update CHANGELOG.md and CONTRIBUTORS.md ISSUES CLOSED: #9817 --- CHANGELOG.md | 8 ++ CONTRIBUTORS.md | 1 + features/plan_cli_coverage_boost.feature | 11 +++ .../steps/plan_cli_coverage_boost_steps.py | 67 +++++++++++++ src/cleveragents/cli/commands/plan.py | 93 ++++++++++++++++++- 5 files changed, 178 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 082e0f8df..0a5d97e1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +-**plan apply --format json output wrapped in spec-required JSON envelope** (PR #9817): + The ``agents plan apply --format json`` command now produces a properly structured + JSON envelope with all required fields: ``command`` ("plan apply"), ``status``, + ``exit_code``, ``data``, ``timing``, and ``messages``. Previously the output used + raw plan data without the spec-required envelope wrapper, making it inconsistent + with ``agents plan execute --format json`` and other API endpoints that follow + the Output Rendering Framework specification (§Output Rendering Framework). + - **Cross-actor subgraph cycle detection reads actor_ref field** (#1431): Fixed `_detect_subgraph_cycles()`, `_map_node()`, and the `compile_actor()` main loop in `src/cleveragents/actor/compiler.py` to read `actor_ref` from the top-level diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 1b5c41879..7b8b4fcfb 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -31,3 +31,4 @@ Below are some of the specific details of various contributions. * HAL 9000 has contributed comprehensive milestone documentation for v3.6.0 (Advanced Concepts & Deferred Features) and v3.7.0 (TUI Implementation) (PR #9903): split into sub-documents covering context strategies, LLM backends, resource types, A2A rename, container tool execution, scope chain resolution, cost/safety budgets, E2E workflow tests, code review examples, plugin architecture, TUI layout, persona system, reference/command input, session management, configuration, and TuiMaterializer integration. * HAL 9000 has contributed the LLMTraceRepository data-integrity fix (PR #8185 / issue #7505): replaced the unconditional `session.commit()` in `LLMTraceRepository.save()` with a dual-path implementation that respects the UnitOfWork pattern — flushing only when an external session is provided, and flushing + committing + closing when operating standalone. This eliminates premature transaction commits, loss of rollback capability, and a docstring/implementation mismatch. * HAL 9000 has contributed the ACMS Index Data Model and File Traversal Engine (PR #9664 / issue #9579): foundational data structures for indexed context entries with hot/warm/cold/archive storage tier classification, tag system, and a timeout-safe chunked file traversal engine for large projects with 10,000+ files. +* HAL 9000 has contributed the `plan apply --format json` spec-required JSON envelope fix (PR #9817): added `_apply_output_dict()` function to build the structured JSON envelope with all required fields (`command`, `status`, `exit_code`, `data`, `timing`, `messages`) for ``agents plan apply --format json``, matching the pattern used by ``plan execute``. Includes BDD test coverage for envelope structure verification. diff --git a/features/plan_cli_coverage_boost.feature b/features/plan_cli_coverage_boost.feature index ca5bf71c1..3f5016d10 100644 --- a/features/plan_cli_coverage_boost.feature +++ b/features/plan_cli_coverage_boost.feature @@ -112,6 +112,17 @@ Feature: Plan CLI coverage boost Then the plan coverage command should succeed And the plan coverage output should contain "plan_id" + @tdd_issue @tdd_issue_9817 + Scenario: apply_plan JSON output has spec-required envelope structure + Given a plan lifecycle CLI runner for coverage + And a mocked lifecycle service for plan coverage commands + And the service has a complete execute plan for apply + When I invoke apply with "--format" "json" and plan id + Then the plan coverage command should succeed + And the apply JSON output has the spec-required envelope fields + And the apply JSON output data has plan_id field + And the apply JSON output data has namespaced_name field + # ---- list_plans regex and state/processing_state filtering ---- Scenario: list_plans filters by regex pattern diff --git a/features/steps/plan_cli_coverage_boost_steps.py b/features/steps/plan_cli_coverage_boost_steps.py index 407d56c4f..ccdd53257 100644 --- a/features/steps/plan_cli_coverage_boost_steps.py +++ b/features/steps/plan_cli_coverage_boost_steps.py @@ -698,3 +698,70 @@ def step_execute_json_progress_list(context) -> None: ) assert "label" in step, f"Expected 'label' key in progress step: {step}" assert "status" in step, f"Expected 'status' key in progress step: {step}" + + +# -------------------------------------------------------------------------- +# apply_plan JSON envelope step definitions (PR #9817) +# -------------------------------------------------------------------------- + + +@then("the apply JSON output has the spec-required envelope fields") +def step_apply_json_envelope_fields(context) -> None: + """Verify the apply JSON output has the spec-required top-level envelope.""" + import json + + output = _output(context) + # The output may have a trailing newline; strip it + parsed = json.loads(output.strip()) + required_fields = {"command", "status", "exit_code", "data", "timing", "messages"} + missing = required_fields - set(parsed.keys()) + assert not missing, ( + f"Missing envelope fields {missing} in apply JSON output:\n{output}" + ) + assert parsed["command"] == "plan apply", ( + f"Expected command='plan apply', got '{parsed['command']}'" + ) + assert parsed["status"] == "ok", f"Expected status='ok', got '{parsed['status']}'" + assert parsed["exit_code"] == 0, ( + f"Expected exit_code=0, got '{parsed['exit_code']}'" + ) + assert isinstance(parsed["data"], dict), ( + f"Expected data to be a dict, got {type(parsed['data'])}" + ) + assert isinstance(parsed["messages"], list), ( + f"Expected messages to be a list, got {type(parsed['messages'])}" + ) + # Verify messages have level and text keys + for msg in parsed["messages"]: + assert "level" in msg, f"Message missing 'level' key: {msg}" + assert "text" in msg, f"Message missing 'text' key: {msg}" + + +@then("the apply JSON output data has plan_id field") +def step_apply_json_data_plan_id(context) -> None: + """Verify the apply JSON output data contains plan_id.""" + import json + + output = _output(context) + parsed = json.loads(output.strip()) + data = parsed["data"] + assert "plan_id" in data, f"Expected 'plan_id' key in apply JSON data: {data}" + assert isinstance(data["plan_id"], str), ( + f"Expected plan_id to be str, got {type(data['plan_id'])}: {data['plan_id']}" + ) + + +@then("the apply JSON output data has namespaced_name field") +def step_apply_json_data_namespaced_name(context) -> None: + """Verify the apply JSON output data contains namespaced_name.""" + import json + + output = _output(context) + parsed = json.loads(output.strip()) + data = parsed["data"] + assert "namespaced_name" in data, ( + f"Expected 'namespaced_name' key in apply JSON data: {data}" + ) + assert isinstance(data["namespaced_name"], str), ( + f"Expected namespaced_name to be str: {data['namespaced_name']}" + ) diff --git a/src/cleveragents/cli/commands/plan.py b/src/cleveragents/cli/commands/plan.py index b2a7f64d0..67589bf46 100644 --- a/src/cleveragents/cli/commands/plan.py +++ b/src/cleveragents/cli/commands/plan.py @@ -477,6 +477,95 @@ def _execute_output_dict( } +def _apply_output_dict( + plan: Any, +) -> dict[str, object]: + """Build the spec-required apply output envelope. + + Returns the structured JSON envelope for ``agents plan apply --format json`` + as defined in the specification §agents plan apply. + + The envelope structure is:: + + { + "command": "plan apply", + "status": "ok", + "exit_code": 0, + "data": { + "plan_id": "...", + "namespaced_name": "...", + "phase": "apply", + "processing_state": "applied", + "project_links": [...] + }, + "timing": {"duration_ms": ...}, + "messages": [{"level": "ok", "text": "Plan applied"}] + } + + Args: + plan: The ``Plan`` domain model after successful application. + + Returns: + A JSON-serialisable dict matching the spec-required envelope. + """ + from cleveragents.domain.models.core.plan import Plan as LifecyclePlan + + if not isinstance(plan, LifecyclePlan): + # Legacy plan fallback — return minimal envelope + return { + "command": "plan apply", + "status": "ok", + "exit_code": 0, + "data": {"plan": str(plan)}, + "timing": {}, + "messages": [{"level": "ok", "text": "Plan applied"}], + } + + plan_id = plan.identity.plan_id + + # ── Project links ──────────────────────────────────────────────────────── + project_links: list[dict[str, object]] = [ + { + "project_name": link.project_name, + **({"alias": link.alias} if link.alias else {}), + **({"read_only": True} if link.read_only else {}), + } + for link in plan.project_links + ] + + # ── Data payload ───────────────────────────────────────────────────────── + data: dict[str, object] = { + "plan_id": plan_id, + "namespaced_name": str(plan.namespaced_name), + "phase": plan.phase.value, + "processing_state": plan.processing_state.value, + "state": plan.processing_state.value if plan.state else "unknown", + "project_links": project_links, + "arguments": plan.arguments, + "automation_profile": ( + plan.automation_profile.profile_name + if plan.automation_profile + else None + ), + "action_name": plan.action_name, + "description": plan.description, + "definition_of_done": plan.definition_of_done, + } + if plan.is_terminal: + data["is_terminal"] = True + if plan.error_message: + data["error_message"] = plan.error_message + + return { + "command": "plan apply", + "status": "ok", + "exit_code": 0, + "data": data, + "timing": {}, + "messages": [{"level": "ok", "text": "Plan applied"}], + } + + def _get_current_project() -> Project: """Get the current project or exit with error. @@ -2271,8 +2360,8 @@ def lifecycle_apply_plan( plan = service._complete_apply_if_queued(plan_id) if fmt != OutputFormat.RICH.value: - data = _plan_spec_dict(plan) - console.print(format_output(data, fmt)) + envelope = _apply_output_dict(plan) + console.print(format_output(envelope, fmt)) else: title = "Plan Applied" if plan.is_terminal else "Plan Applying" _print_lifecycle_plan(plan, title=title)