diff --git a/CHANGELOG.md b/CHANGELOG.md index 5edafb3f8..7c413000c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -389,6 +389,19 @@ ensuring data is stored with proper parameter values. the assertion to validate envelope structure and removed `@tdd_expected_fail` from the `@tdd_issue_4254` scenario so it runs as a permanent regression guard. The code producing `decision_id` in tree nodes was already correct; only the test assertion needed fixing. +- **`agents plan apply --format json` now returns spec-required JSON envelope** (#9449): + Fixed `lifecycle_apply_plan` in `src/cleveragents/cli/commands/plan.py` to wrap + non-rich format output in the spec-required JSON envelope instead of a raw plan + dictionary. The envelope includes `command: "plan apply"`, `status: "ok"`, + `exit_code: 0`, and `timing` fields. The `data` field contains `artifacts` (count + of sandbox refs), `changes` (insertions/deletions from git diff), `project` (first + project link), `applied_at` (ISO timestamp), `validation` (test/lint/type_check + results with duration), `sandbox_cleanup` (worktree/branch/checkpoint status derived + from actual plan terminal state), and `lifecycle` (phase, state, total_duration, + total_cost, decisions_made, child_plans). The `Plan as LifecyclePlan` import was moved + to the module top-level. The new `_apply_output_dict` helper is scoped exclusively to + `plan apply` call sites; all other commands (`plan status`, `plan cancel`, `plan + revert`, `plan use`) continue to use `_plan_spec_dict` unchanged. ### Changed @@ -1313,4 +1326,4 @@ iteration` and data corruption under concurrent plan execution. All public - **TUI -- Permission Question Widget**: A new inline `PermissionQuestionWidget` renders permission requests directly in the conversation stream for single-key operations. Users can allow/reject with single-key shortcuts (`a`/`A`/`r`/`R`), - navigate with arrow keys, confirm with `Enter`, or press `v` to open the full \ No newline at end of file + navigate with arrow keys, confirm with `Enter`, or press `v` to open the full diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index f64eaebf1..177bae6f4 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -75,3 +75,4 @@ Below are some of the specific details of various contributions. * HAL 9000 has contributed the `--clone-into` CLI argument for `container-instance`, the `CloneIntoHandler` module, the `devcontainer-instance` snapshot sandbox strategy, and the `ContainerLifecycleState.DISCOVERED` terminology alignment (PR #8304, issue #7555). * HAL 9000 has contributed the ACMS Context Tier Hydration documentation (PR #9208 / issue #6175): documented the `context_tier_hydrator` module in the ACMS Architecture section of the specification, covering its public interface, file listing strategy, budget limits, and fragment structure. * HAL 9000 has contributed the agent task memory leak fix (#9044): replaced `list.remove` with `set.discard` as the done_callback for asyncio tasks in `Agent._tasks`, preventing unbounded memory growth in long-lived agents and ensuring safe concurrent task removal. +* HAL 9000 has contributed the `plan apply --format json` spec-compliant envelope fix (PR #9817 / issue #9449): replaced raw plan dictionary output with a spec-required JSON envelope containing structured data fields for artifacts, changes, validation, sandbox cleanup, and lifecycle metrics across all output formats. Full BDD test suite in behave + Robot Framework integration tests added. diff --git a/features/plan_apply_json_envelope.feature b/features/plan_apply_json_envelope.feature new file mode 100644 index 000000000..b7cbb803e --- /dev/null +++ b/features/plan_apply_json_envelope.feature @@ -0,0 +1,105 @@ +Feature: plan apply --format json returns spec-required JSON envelope + As a programmatic consumer of the CleverAgents CLI + I want ``agents plan apply --format json`` to return a spec-compliant JSON envelope + So that tooling and scripts can reliably parse the apply output + + Background: + Given a plan CLI runner for the apply JSON envelope test + + # ── Envelope field presence ────────────────────────────────────────────── + + Scenario: apply JSON output includes all required top-level envelope fields + When I invoke plan apply with --format json and a mocked service + Then the apply JSON envelope should contain field "command" + And the apply JSON envelope should contain field "status" + And the apply JSON envelope should contain field "exit_code" + And the apply JSON envelope should contain field "data" + And the apply JSON envelope should contain field "timing" + And the apply JSON envelope should contain field "messages" + + Scenario: apply JSON envelope command field is "plan apply" + When I invoke plan apply with --format json and a mocked service + Then the apply JSON envelope command should be "plan apply" + + Scenario: apply JSON envelope status field is "ok" + When I invoke plan apply with --format json and a mocked service + Then the apply JSON envelope status should be "ok" + + Scenario: apply JSON envelope exit_code field is 0 + When I invoke plan apply with --format json and a mocked service + Then the apply JSON envelope exit_code should be 0 + + Scenario: apply JSON envelope messages is a non-empty list + When I invoke plan apply with --format json and a mocked service + Then the apply JSON envelope messages should be a non-empty list + + # ── Data field structure ───────────────────────────────────────────────── + + Scenario: apply JSON envelope data field contains required sub-fields + When I invoke plan apply with --format json and a mocked service + Then the apply JSON data should contain field "artifacts" + And the apply JSON data should contain field "changes" + And the apply JSON data should contain field "project" + And the apply JSON data should contain field "applied_at" + And the apply JSON data should contain field "validation" + And the apply JSON data should contain field "sandbox_cleanup" + And the apply JSON data should contain field "lifecycle" + + Scenario: apply JSON data validation field contains tests lint type_check and duration_s + When I invoke plan apply with --format json and a mocked service + Then the apply JSON data validation should contain field "tests" + And the apply JSON data validation should contain field "lint" + And the apply JSON data validation should contain field "type_check" + And the apply JSON data validation should contain field "duration_s" + + Scenario: apply JSON data lifecycle field contains required sub-fields + When I invoke plan apply with --format json and a mocked service + Then the apply JSON data lifecycle should contain field "phase" + And the apply JSON data lifecycle should contain field "state" + And the apply JSON data lifecycle should contain field "total_duration" + And the apply JSON data lifecycle should contain field "total_cost" + And the apply JSON data lifecycle should contain field "decisions_made" + And the apply JSON data lifecycle should contain field "child_plans" + + Scenario: apply JSON data sandbox_cleanup field contains worktree branch checkpoint + When I invoke plan apply with --format json and a mocked service + Then the apply JSON data sandbox_cleanup should contain field "worktree" + And the apply JSON data sandbox_cleanup should contain field "branch" + And the apply JSON data sandbox_cleanup should contain field "checkpoint" + + # ── sandbox_cleanup derives from actual plan state ─────────────────────── + + Scenario: sandbox_cleanup shows removed state for terminal apply plan + When I invoke plan apply with --format json and a terminal apply plan + Then the apply JSON data sandbox_cleanup worktree should be "removed" + And the apply JSON data sandbox_cleanup branch should be "merged to main" + And the apply JSON data sandbox_cleanup checkpoint should be "archived" + + Scenario: sandbox_cleanup shows active state for non-terminal plan + When I invoke plan apply with --format json and a non-terminal plan + Then the apply JSON data sandbox_cleanup worktree should be "active" + And the apply JSON data sandbox_cleanup branch should be "open" + And the apply JSON data sandbox_cleanup checkpoint should be "pending" + + # ── Command isolation — other commands must not use apply envelope ──────── + + Scenario: plan status --format json does not emit plan apply command field + When I invoke plan status with --format json and a mocked service + Then the plan status JSON output should not have command "plan apply" + + Scenario: plan cancel --format json does not emit plan apply command field + When I invoke plan cancel with --format json and a mocked service + Then the plan cancel JSON output should not have command "plan apply" + + # ── Legacy plan fallback ───────────────────────────────────────────────── + + Scenario: apply JSON envelope handles legacy plan object gracefully + When I invoke the apply output dict with a legacy plan object + Then the legacy apply JSON envelope command should be "plan apply" + And the legacy apply JSON envelope data should contain field "plan" + + # ── total_cost from cost_metadata ──────────────────────────────────────── + + Scenario: apply JSON data lifecycle total_cost reflects plan cost_metadata + When I invoke plan apply with --format json and a plan with cost metadata + Then the apply JSON data lifecycle total_cost should start with "$" diff --git a/features/steps/plan_apply_json_envelope_steps.py b/features/steps/plan_apply_json_envelope_steps.py new file mode 100644 index 000000000..b0132d0ef --- /dev/null +++ b/features/steps/plan_apply_json_envelope_steps.py @@ -0,0 +1,441 @@ +"""Step definitions for plan apply --format json JSON envelope tests. + +These steps verify that ``agents plan apply --format json`` returns the +spec-required JSON envelope with ``command``, ``status``, ``exit_code``, +``data``, ``timing``, and ``messages`` fields, and that the ``data`` field +contains the required sub-fields for artifacts, changes, project, applied_at, +validation, sandbox_cleanup, and lifecycle. +""" + +from __future__ import annotations + +import json +from datetime import UTC, datetime +from unittest.mock import MagicMock, patch + +from behave import given, then, when +from behave.runner import Context +from typer.testing import CliRunner + +from cleveragents.cli.commands.plan import _apply_output_dict +from cleveragents.cli.commands.plan import app as plan_app +from cleveragents.domain.models.core.cost_metadata import CostMetadata +from cleveragents.domain.models.core.plan import ( + NamespacedName, + Plan as LifecyclePlan, + PlanIdentity, + PlanPhase, + PlanTimestamps, + ProcessingState, + ProjectLink, +) + +_PATCH_LIFECYCLE = "cleveragents.cli.commands.plan._get_lifecycle_service" +_PATCH_SANDBOX = "cleveragents.cli.commands.plan._apply_sandbox_changes" + +_PLAN_ID = "01JAAAAAAAAAAAAAAAAAAAAAAA" + + +def _make_plan( + phase: PlanPhase = PlanPhase.APPLY, + state: ProcessingState = ProcessingState.APPLIED, + cost_metadata: CostMetadata | None = None, +) -> LifecyclePlan: + """Build a lightweight Plan for apply JSON envelope tests.""" + now = datetime.now(tz=UTC) + return LifecyclePlan( + identity=PlanIdentity(plan_id=_PLAN_ID), + namespaced_name=NamespacedName( + server=None, namespace="local", name="test-plan" + ), + action_name="local/test-action", + description="Test plan for apply JSON envelope scenarios", + definition_of_done="All tests pass", + phase=phase, + processing_state=state, + project_links=[ProjectLink(project_name="local/my-project")], + strategy_actor="openai/gpt-4", + execution_actor="openai/gpt-4", + created_by="test-user", + reusable=True, + read_only=False, + timestamps=PlanTimestamps( + created_at=now, + updated_at=now, + applied_at=now, + ), + cost_metadata=cost_metadata, + ) + + +def _build_mock_service( + plan: LifecyclePlan | None = None, +) -> MagicMock: + """Build a mock lifecycle service for apply JSON envelope tests.""" + if plan is None: + plan = _make_plan() + mock_service = MagicMock() + mock_service.get_plan.return_value = plan + mock_service.apply_plan.return_value = plan + mock_service._complete_apply_if_queued.return_value = plan + return mock_service + + +def _invoke_apply_json( + context: Context, + plan: LifecyclePlan | None = None, +) -> None: + """Invoke ``plan apply --yes --format json`` with a mocked service.""" + mock_service = _build_mock_service(plan) + context.apply_json_mock_service = mock_service + + with ( + patch(_PATCH_LIFECYCLE, return_value=mock_service), + patch(_PATCH_SANDBOX, return_value=True), + ): + result = context.apply_json_runner.invoke( + plan_app, + ["apply", "--yes", "--format", "json", _PLAN_ID], + ) + context.apply_json_result = result + # Parse the JSON output — strip any Rich markup / ANSI codes. + raw = result.output.strip() + try: + context.apply_json_envelope = json.loads(raw) + except json.JSONDecodeError: + context.apply_json_envelope = None + + +# --------------------------------------------------------------------------- +# GIVEN +# --------------------------------------------------------------------------- + + +@given("a plan CLI runner for the apply JSON envelope test") +def step_given_runner(context: Context) -> None: + """Create a Typer CliRunner for the plan sub-app.""" + context.apply_json_runner = CliRunner() + + +# --------------------------------------------------------------------------- +# WHEN +# --------------------------------------------------------------------------- + + +@when("I invoke plan apply with --format json and a mocked service") +def step_when_invoke_apply_json(context: Context) -> None: + """Invoke ``plan apply --yes --format json`` with a standard mocked service.""" + _invoke_apply_json(context) + + +@when("I invoke plan apply with --format json and a terminal apply plan") +def step_when_invoke_apply_json_terminal(context: Context) -> None: + """Invoke apply with a plan in terminal Apply/applied state.""" + plan = _make_plan(phase=PlanPhase.APPLY, state=ProcessingState.APPLIED) + _invoke_apply_json(context, plan) + + +@when("I invoke plan apply with --format json and a non-terminal plan") +def step_when_invoke_apply_json_non_terminal(context: Context) -> None: + """Invoke apply with a plan in non-terminal Apply/queued state.""" + plan = _make_plan(phase=PlanPhase.APPLY, state=ProcessingState.QUEUED) + _invoke_apply_json(context, plan) + + +@when("I invoke plan apply with --format json and a plan with cost metadata") +def step_when_invoke_apply_json_cost(context: Context) -> None: + """Invoke apply with a plan that has cost_metadata set.""" + cost = CostMetadata(total_cost=1.23) + plan = _make_plan(cost_metadata=cost) + _invoke_apply_json(context, plan) + + +@when("I invoke plan status with --format json and a mocked service") +def step_when_invoke_status_json(context: Context) -> None: + """Invoke ``plan status --format json`` with a mocked service.""" + plan = _make_plan() + mock_service = _build_mock_service(plan) + mock_service.list_plans.return_value = [plan] + + with patch(_PATCH_LIFECYCLE, return_value=mock_service): + result = context.apply_json_runner.invoke( + plan_app, + ["status", _PLAN_ID, "--format", "json"], + ) + context.plan_status_json_result = result + raw = result.output.strip() + try: + context.plan_status_json_envelope = json.loads(raw) + except json.JSONDecodeError: + context.plan_status_json_envelope = None + + +@when("I invoke plan cancel with --format json and a mocked service") +def step_when_invoke_cancel_json(context: Context) -> None: + """Invoke ``plan cancel --format json`` with a mocked service.""" + plan = _make_plan(phase=PlanPhase.APPLY, state=ProcessingState.CANCELLED) + mock_service = _build_mock_service(plan) + mock_service.cancel_plan.return_value = plan + + with patch(_PATCH_LIFECYCLE, return_value=mock_service): + result = context.apply_json_runner.invoke( + plan_app, + ["cancel", _PLAN_ID, "--format", "json"], + ) + context.plan_cancel_json_result = result + raw = result.output.strip() + try: + context.plan_cancel_json_envelope = json.loads(raw) + except json.JSONDecodeError: + context.plan_cancel_json_envelope = None + + +@when("I invoke the apply output dict with a legacy plan object") +def step_when_invoke_apply_output_dict_legacy(context: Context) -> None: + """Call ``_apply_output_dict`` directly with a non-Plan object.""" + context.legacy_apply_envelope = _apply_output_dict("legacy-plan-string") + + +# --------------------------------------------------------------------------- +# THEN — envelope field presence +# --------------------------------------------------------------------------- + + +@then('the apply JSON envelope should contain field "{field}"') +def step_then_envelope_has_field(context: Context, field: str) -> None: + """Assert the top-level envelope contains the given field.""" + envelope = context.apply_json_envelope + assert envelope is not None, ( + f"Could not parse JSON output.\nRaw output:\n" + f"{context.apply_json_result.output}" + ) + assert field in envelope, ( + f"Expected envelope to contain field '{field}'.\n" + f"Envelope keys: {list(envelope.keys())}" + ) + + +@then('the apply JSON envelope command should be "{expected}"') +def step_then_envelope_command(context: Context, expected: str) -> None: + """Assert the envelope ``command`` field equals the expected value.""" + envelope = context.apply_json_envelope + assert envelope is not None, "Could not parse JSON output." + assert envelope.get("command") == expected, ( + f"Expected command '{expected}', got '{envelope.get('command')}'." + ) + + +@then('the apply JSON envelope status should be "{expected}"') +def step_then_envelope_status(context: Context, expected: str) -> None: + """Assert the envelope ``status`` field equals the expected value.""" + envelope = context.apply_json_envelope + assert envelope is not None, "Could not parse JSON output." + assert envelope.get("status") == expected, ( + f"Expected status '{expected}', got '{envelope.get('status')}'." + ) + + +@then("the apply JSON envelope exit_code should be {expected:d}") +def step_then_envelope_exit_code(context: Context, expected: int) -> None: + """Assert the envelope ``exit_code`` field equals the expected value.""" + envelope = context.apply_json_envelope + assert envelope is not None, "Could not parse JSON output." + assert envelope.get("exit_code") == expected, ( + f"Expected exit_code {expected}, got '{envelope.get('exit_code')}'." + ) + + +@then("the apply JSON envelope messages should be a non-empty list") +def step_then_envelope_messages_nonempty(context: Context) -> None: + """Assert the envelope ``messages`` field is a non-empty list.""" + envelope = context.apply_json_envelope + assert envelope is not None, "Could not parse JSON output." + messages = envelope.get("messages") + assert isinstance(messages, list) and len(messages) > 0, ( + f"Expected messages to be a non-empty list, got: {messages!r}" + ) + + +# --------------------------------------------------------------------------- +# THEN — data field structure +# --------------------------------------------------------------------------- + + +@then('the apply JSON data should contain field "{field}"') +def step_then_data_has_field(context: Context, field: str) -> None: + """Assert the ``data`` sub-dict contains the given field.""" + envelope = context.apply_json_envelope + assert envelope is not None, "Could not parse JSON output." + data = envelope.get("data", {}) + assert field in data, ( + f"Expected data to contain field '{field}'.\n" + f"Data keys: {list(data.keys())}" + ) + + +@then('the apply JSON data validation should contain field "{field}"') +def step_then_data_validation_has_field(context: Context, field: str) -> None: + """Assert the ``data.validation`` sub-dict contains the given field.""" + envelope = context.apply_json_envelope + assert envelope is not None, "Could not parse JSON output." + validation = envelope.get("data", {}).get("validation", {}) + assert field in validation, ( + f"Expected data.validation to contain field '{field}'.\n" + f"Validation keys: {list(validation.keys())}" + ) + + +@then('the apply JSON data lifecycle should contain field "{field}"') +def step_then_data_lifecycle_has_field(context: Context, field: str) -> None: + """Assert the ``data.lifecycle`` sub-dict contains the given field.""" + envelope = context.apply_json_envelope + assert envelope is not None, "Could not parse JSON output." + lifecycle = envelope.get("data", {}).get("lifecycle", {}) + assert field in lifecycle, ( + f"Expected data.lifecycle to contain field '{field}'.\n" + f"Lifecycle keys: {list(lifecycle.keys())}" + ) + + +@then('the apply JSON data sandbox_cleanup should contain field "{field}"') +def step_then_data_sandbox_cleanup_has_field( + context: Context, field: str +) -> None: + """Assert the ``data.sandbox_cleanup`` sub-dict contains the given field.""" + envelope = context.apply_json_envelope + assert envelope is not None, "Could not parse JSON output." + sandbox_cleanup = envelope.get("data", {}).get("sandbox_cleanup", {}) + assert field in sandbox_cleanup, ( + f"Expected data.sandbox_cleanup to contain field '{field}'.\n" + f"sandbox_cleanup keys: {list(sandbox_cleanup.keys())}" + ) + + +# --------------------------------------------------------------------------- +# THEN — sandbox_cleanup state values +# --------------------------------------------------------------------------- + + +@then('the apply JSON data sandbox_cleanup worktree should be "{expected}"') +def step_then_sandbox_cleanup_worktree( + context: Context, expected: str +) -> None: + """Assert the ``data.sandbox_cleanup.worktree`` value.""" + envelope = context.apply_json_envelope + assert envelope is not None, "Could not parse JSON output." + actual = envelope.get("data", {}).get("sandbox_cleanup", {}).get("worktree") + assert actual == expected, ( + f"Expected sandbox_cleanup.worktree '{expected}', got '{actual}'." + ) + + +@then('the apply JSON data sandbox_cleanup branch should be "{expected}"') +def step_then_sandbox_cleanup_branch(context: Context, expected: str) -> None: + """Assert the ``data.sandbox_cleanup.branch`` value.""" + envelope = context.apply_json_envelope + assert envelope is not None, "Could not parse JSON output." + actual = ( + envelope.get("data", {}).get("sandbox_cleanup", {}).get("branch") + ) + assert actual == expected, ( + f"Expected sandbox_cleanup.branch '{expected}', got '{actual}'." + ) + + +@then('the apply JSON data sandbox_cleanup checkpoint should be "{expected}"') +def step_then_sandbox_cleanup_checkpoint( + context: Context, expected: str +) -> None: + """Assert the ``data.sandbox_cleanup.checkpoint`` value.""" + envelope = context.apply_json_envelope + assert envelope is not None, "Could not parse JSON output." + actual = ( + envelope.get("data", {}).get("sandbox_cleanup", {}).get("checkpoint") + ) + assert actual == expected, ( + f"Expected sandbox_cleanup.checkpoint '{expected}', got '{actual}'." + ) + + +# --------------------------------------------------------------------------- +# THEN — command isolation +# --------------------------------------------------------------------------- + + +@then('the plan status JSON output should not have command "plan apply"') +def step_then_status_not_apply_command(context: Context) -> None: + """Assert plan status JSON output does not emit ``command: plan apply``.""" + envelope = context.plan_status_json_envelope + if envelope is None: + # If output is not JSON (e.g., plain dict), check raw output + raw = context.plan_status_json_result.output + assert '"plan apply"' not in raw, ( + f"plan status output should not contain 'plan apply'.\n" + f"Output:\n{raw}" + ) + return + command = envelope.get("command", "") + assert command != "plan apply", ( + f"plan status should not emit command 'plan apply', got '{command}'." + ) + + +@then('the plan cancel JSON output should not have command "plan apply"') +def step_then_cancel_not_apply_command(context: Context) -> None: + """Assert plan cancel JSON output does not emit ``command: plan apply``.""" + envelope = context.plan_cancel_json_envelope + if envelope is None: + raw = context.plan_cancel_json_result.output + assert '"plan apply"' not in raw, ( + f"plan cancel output should not contain 'plan apply'.\n" + f"Output:\n{raw}" + ) + return + command = envelope.get("command", "") + assert command != "plan apply", ( + f"plan cancel should not emit command 'plan apply', got '{command}'." + ) + + +# --------------------------------------------------------------------------- +# THEN — legacy plan fallback +# --------------------------------------------------------------------------- + + +@then('the legacy apply JSON envelope command should be "{expected}"') +def step_then_legacy_envelope_command(context: Context, expected: str) -> None: + """Assert the legacy fallback envelope ``command`` field.""" + envelope = context.legacy_apply_envelope + assert envelope.get("command") == expected, ( + f"Expected legacy envelope command '{expected}', " + f"got '{envelope.get('command')}'." + ) + + +@then('the legacy apply JSON envelope data should contain field "{field}"') +def step_then_legacy_data_has_field(context: Context, field: str) -> None: + """Assert the legacy fallback envelope ``data`` contains the given field.""" + envelope = context.legacy_apply_envelope + data = envelope.get("data", {}) + assert field in data, ( + f"Expected legacy data to contain field '{field}'.\n" + f"Data keys: {list(data.keys())}" + ) + + +# --------------------------------------------------------------------------- +# THEN — total_cost +# --------------------------------------------------------------------------- + + +@then('the apply JSON data lifecycle total_cost should start with "$"') +def step_then_total_cost_dollar(context: Context) -> None: + """Assert the ``data.lifecycle.total_cost`` starts with a dollar sign.""" + envelope = context.apply_json_envelope + assert envelope is not None, "Could not parse JSON output." + total_cost = ( + envelope.get("data", {}).get("lifecycle", {}).get("total_cost", "") + ) + assert str(total_cost).startswith("$"), ( + f"Expected total_cost to start with '$', got '{total_cost}'." + ) diff --git a/robot/helper_plan_apply_json_envelope.py b/robot/helper_plan_apply_json_envelope.py new file mode 100644 index 000000000..14841fe12 --- /dev/null +++ b/robot/helper_plan_apply_json_envelope.py @@ -0,0 +1,373 @@ +"""Helper script for plan_apply_json_envelope.robot smoke tests. + +Each subcommand exercises the real CLI path (with mocked services) to verify +that ``agents plan apply --format json`` returns the spec-required JSON +envelope structure. + +The helper reports the **real** outcome: it exits 0 and prints the sentinel +when the envelope is correct, and exits 1 with an error message when it is not. +""" + +# ruff: noqa: E402, I001 +from __future__ import annotations + +import json +import sys +from collections.abc import Callable +from datetime import UTC, datetime +from pathlib import Path +from typing import NoReturn +from unittest.mock import MagicMock, patch + +# Ensure local source tree is importable. +_ROOT = Path(__file__).resolve().parents[1] +_SRC = str(_ROOT / "src") +if _SRC not in sys.path: + sys.path.insert(0, _SRC) + +from typer.testing import CliRunner + +from cleveragents.cli.commands.plan import app as plan_app +from cleveragents.domain.models.core.plan import ( + NamespacedName, + Plan as LifecyclePlan, + PlanIdentity, + PlanPhase, + PlanTimestamps, + ProcessingState, + ProjectLink, +) + +runner = CliRunner() + +_PLAN_ID = "01JAAAAAAAAAAAAAAAAAAAAAAA" +_PATCH_LIFECYCLE = "cleveragents.cli.commands.plan._get_lifecycle_service" +_PATCH_SANDBOX = "cleveragents.cli.commands.plan._apply_sandbox_changes" + + +def _fail(message: str) -> NoReturn: + """Print an error message to stderr and exit with code 1.""" + print(message, file=sys.stderr) + sys.exit(1) + + +def _make_plan( + phase: PlanPhase = PlanPhase.APPLY, + state: ProcessingState = ProcessingState.APPLIED, +) -> LifecyclePlan: + """Build a lightweight Plan for apply JSON envelope tests.""" + now = datetime.now(tz=UTC) + return LifecyclePlan( + identity=PlanIdentity(plan_id=_PLAN_ID), + namespaced_name=NamespacedName( + server=None, namespace="local", name="test-plan" + ), + action_name="local/test-action", + description="Test plan for apply JSON envelope scenarios", + definition_of_done="All tests pass", + phase=phase, + processing_state=state, + project_links=[ProjectLink(project_name="local/my-project")], + strategy_actor="openai/gpt-4", + execution_actor="openai/gpt-4", + created_by="test-user", + reusable=True, + read_only=False, + timestamps=PlanTimestamps( + created_at=now, + updated_at=now, + applied_at=now, + ), + ) + + +def _build_mock_service(plan: LifecyclePlan | None = None) -> MagicMock: + """Build a mock lifecycle service for apply JSON envelope tests.""" + if plan is None: + plan = _make_plan() + mock_service = MagicMock() + mock_service.get_plan.return_value = plan + mock_service.apply_plan.return_value = plan + mock_service._complete_apply_if_queued.return_value = plan + return mock_service + + +def _invoke_apply_json(plan: LifecyclePlan | None = None) -> dict[str, object]: + """Invoke ``plan apply --yes --format json`` and return parsed envelope.""" + mock_service = _build_mock_service(plan) + with ( + patch(_PATCH_LIFECYCLE, return_value=mock_service), + patch(_PATCH_SANDBOX, return_value=True), + ): + result = runner.invoke( + plan_app, + ["apply", "--yes", "--format", "json", _PLAN_ID], + ) + raw = result.output.strip() + try: + return json.loads(raw) + except json.JSONDecodeError as exc: + _fail( + f"Could not parse JSON output.\n" + f"Exit code: {result.exit_code}\n" + f"Output: {result.output}\n" + f"Error: {exc}" + ) + + +# --------------------------------------------------------------------------- +# Subcommands +# --------------------------------------------------------------------------- + + +def check_envelope_fields() -> None: + """Verify the apply JSON envelope contains all required top-level fields.""" + envelope = _invoke_apply_json() + required = {"command", "status", "exit_code", "data", "timing", "messages"} + missing = required - set(envelope.keys()) + if missing: + _fail( + f"apply JSON envelope missing required fields: {missing}\n" + f"Envelope keys: {list(envelope.keys())}" + ) + print("plan-apply-json-envelope-fields-ok") + + +def check_command_field() -> None: + """Verify the apply JSON envelope command field is 'plan apply'.""" + envelope = _invoke_apply_json() + command = envelope.get("command") + if command != "plan apply": + _fail( + f"Expected envelope command 'plan apply', got '{command}'.\n" + f"Envelope: {envelope}" + ) + print("plan-apply-json-command-ok") + + +def check_data_fields() -> None: + """Verify the apply JSON envelope data field contains required sub-fields.""" + envelope = _invoke_apply_json() + data = envelope.get("data", {}) + required_data = { + "artifacts", + "changes", + "project", + "applied_at", + "validation", + "sandbox_cleanup", + "lifecycle", + } + missing = required_data - set(data.keys()) + if missing: + _fail( + f"apply JSON data missing required fields: {missing}\n" + f"Data keys: {list(data.keys())}" + ) + + # Verify lifecycle sub-fields + lifecycle = data.get("lifecycle", {}) + required_lifecycle = { + "phase", + "state", + "total_duration", + "total_cost", + "decisions_made", + "child_plans", + } + missing_lifecycle = required_lifecycle - set(lifecycle.keys()) + if missing_lifecycle: + _fail( + f"apply JSON data.lifecycle missing required fields: " + f"{missing_lifecycle}\n" + f"Lifecycle keys: {list(lifecycle.keys())}" + ) + + # Verify validation sub-fields + validation = data.get("validation", {}) + required_validation = {"tests", "lint", "type_check", "duration_s"} + missing_validation = required_validation - set(validation.keys()) + if missing_validation: + _fail( + f"apply JSON data.validation missing required fields: " + f"{missing_validation}\n" + f"Validation keys: {list(validation.keys())}" + ) + + # Verify tests sub-fields + tests_sub = data.get("validation", {}).get("tests", {}) + required_tests = {"status", "passed", "total"} + if tests_sub: + missing_tests = required_tests - set(tests_sub.keys()) + if missing_tests: + _fail( + f"apply JSON data.validation.tests missing fields: " + f"{missing_tests}\n" + f"Tests keys: {list(tests_sub.keys())}" + ) + + # Verify lint sub-fields + lint_sub = data.get("validation", {}).get("lint", {}) + required_lint = {"status", "warnings"} + if lint_sub: + missing_lint_keys = required_lint - set(lint_sub.keys()) + if missing_lint_keys: + _fail( + f"apply JSON data.validation.lint missing fields: " + f"{missing_lint_keys}\n" + f"Lint keys: {list(lint_sub.keys())}" + ) + + # Verify type_check sub-fields + type_check_sub = data.get("validation", {}).get("type_check", {}) + required_type_check = {"status", "errors"} + if type_check_sub: + missing_tc = required_type_check - set(type_check_sub.keys()) + if missing_tc: + _fail( + f"apply JSON data.validation.type_check missing fields: " + f"{missing_tc}\n" + f"type_check keys: {list(type_check_sub.keys())}" + ) + + # Verify sandbox_cleanup sub-fields + sandbox_cleanup = data.get("sandbox_cleanup", {}) + required_sandbox = {"worktree", "branch", "checkpoint"} + missing_sandbox = required_sandbox - set(sandbox_cleanup.keys()) + if missing_sandbox: + _fail( + f"apply JSON data.sandbox_cleanup missing required fields: " + f"{missing_sandbox}\n" + f"sandbox_cleanup keys: {list(sandbox_cleanup.keys())}" + ) + + print("plan-apply-json-data-fields-ok") + + +def check_sandbox_cleanup() -> None: + """Verify sandbox_cleanup values reflect actual plan terminal state.""" + # Terminal plan (Apply/applied) → removed/merged/archived + terminal_plan = _make_plan( + phase=PlanPhase.APPLY, state=ProcessingState.APPLIED + ) + terminal_envelope = _invoke_apply_json(terminal_plan) + terminal_cleanup = ( + terminal_envelope.get("data", {}).get("sandbox_cleanup", {}) + ) + if terminal_cleanup.get("worktree") != "removed": + _fail( + f"Expected terminal plan sandbox_cleanup.worktree='removed', " + f"got '{terminal_cleanup.get('worktree')}'." + ) + if terminal_cleanup.get("branch") != "merged to main": + _fail( + f"Expected terminal plan sandbox_cleanup.branch='merged to main', " + f"got '{terminal_cleanup.get('branch')}'." + ) + if terminal_cleanup.get("checkpoint") != "archived": + _fail( + f"Expected terminal plan sandbox_cleanup.checkpoint='archived', " + f"got '{terminal_cleanup.get('checkpoint')}'." + ) + + # Non-terminal plan (Apply/queued) → active/open/pending + non_terminal_plan = _make_plan( + phase=PlanPhase.APPLY, state=ProcessingState.QUEUED + ) + non_terminal_envelope = _invoke_apply_json(non_terminal_plan) + non_terminal_cleanup = ( + non_terminal_envelope.get("data", {}).get("sandbox_cleanup", {}) + ) + if non_terminal_cleanup.get("worktree") != "active": + _fail( + f"Expected non-terminal plan sandbox_cleanup.worktree='active', " + f"got '{non_terminal_cleanup.get('worktree')}'." + ) + if non_terminal_cleanup.get("branch") != "open": + _fail( + f"Expected non-terminal plan sandbox_cleanup.branch='open', " + f"got '{non_terminal_cleanup.get('branch')}'." + ) + if non_terminal_cleanup.get("checkpoint") != "pending": + _fail( + f"Expected non-terminal plan sandbox_cleanup.checkpoint='pending', " + f"got '{non_terminal_cleanup.get('checkpoint')}'." + ) + + print("plan-apply-json-sandbox-cleanup-ok") + + +def check_command_isolation() -> None: + """Verify plan status and cancel do not emit 'plan apply' command field.""" + plan = _make_plan() + mock_service = _build_mock_service(plan) + mock_service.list_plans.return_value = [plan] + + # Check plan status + with patch(_PATCH_LIFECYCLE, return_value=mock_service): + status_result = runner.invoke( + plan_app, + ["status", _PLAN_ID, "--format", "json"], + ) + status_raw = status_result.output.strip() + try: + status_envelope = json.loads(status_raw) + status_command = status_envelope.get("command", "") + except json.JSONDecodeError: + status_command = "" + + if status_command == "plan apply": + _fail( + f"plan status should not emit command 'plan apply', " + f"got '{status_command}'.\nOutput: {status_result.output}" + ) + + # Check plan cancel + cancelled_plan = _make_plan( + phase=PlanPhase.APPLY, state=ProcessingState.CANCELLED + ) + cancel_mock = _build_mock_service(cancelled_plan) + cancel_mock.cancel_plan.return_value = cancelled_plan + + with patch(_PATCH_LIFECYCLE, return_value=cancel_mock): + cancel_result = runner.invoke( + plan_app, + ["cancel", _PLAN_ID, "--format", "json"], + ) + cancel_raw = cancel_result.output.strip() + try: + cancel_envelope = json.loads(cancel_raw) + cancel_command = cancel_envelope.get("command", "") + except json.JSONDecodeError: + cancel_command = "" + + if cancel_command == "plan apply": + _fail( + f"plan cancel should not emit command 'plan apply', " + f"got '{cancel_command}'.\nOutput: {cancel_result.output}" + ) + + print("plan-apply-json-command-isolation-ok") + + +# --------------------------------------------------------------------------- +# Dispatcher +# --------------------------------------------------------------------------- + +_COMMANDS: dict[str, Callable[[], None]] = { + "check-envelope-fields": check_envelope_fields, + "check-command-field": check_command_field, + "check-data-fields": check_data_fields, + "check-sandbox-cleanup": check_sandbox_cleanup, + "check-command-isolation": check_command_isolation, +} + +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 = _COMMANDS[sys.argv[1]] + cmd() diff --git a/robot/plan_apply_json_envelope.robot b/robot/plan_apply_json_envelope.robot new file mode 100644 index 000000000..f7b154da7 --- /dev/null +++ b/robot/plan_apply_json_envelope.robot @@ -0,0 +1,60 @@ +*** Settings *** +Documentation Integration smoke tests for plan apply --format json JSON envelope. +... +... Verifies that ``agents plan apply --format json`` returns the +... spec-required JSON envelope with ``command``, ``status``, +... ``exit_code``, ``data``, ``timing``, and ``messages`` fields. +... Also verifies that ``data`` contains the required sub-fields +... and that sandbox_cleanup derives from actual plan state. +Resource ${CURDIR}/common.resource +Suite Setup Setup Test Environment +Suite Teardown Cleanup Test Environment + +*** Variables *** +${HELPER} ${CURDIR}/helper_plan_apply_json_envelope.py + +*** Test Cases *** +Plan Apply JSON Envelope Has Required Top-Level Fields + [Documentation] Verify the apply JSON envelope contains all required top-level fields. + [Tags] plan_apply json_envelope + ${result}= Run Process ${PYTHON} ${HELPER} check-envelope-fields cwd=${WORKSPACE} timeout=120s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} plan-apply-json-envelope-fields-ok + +Plan Apply JSON Envelope Command Is Plan Apply + [Documentation] Verify the apply JSON envelope command field is "plan apply". + [Tags] plan_apply json_envelope + ${result}= Run Process ${PYTHON} ${HELPER} check-command-field cwd=${WORKSPACE} timeout=120s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} plan-apply-json-command-ok + +Plan Apply JSON Envelope Data Has Required Sub-Fields + [Documentation] Verify the apply JSON envelope data field contains required sub-fields. + [Tags] plan_apply json_envelope + ${result}= Run Process ${PYTHON} ${HELPER} check-data-fields cwd=${WORKSPACE} timeout=120s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} plan-apply-json-data-fields-ok + +Plan Apply JSON Sandbox Cleanup Derives From Plan State + [Documentation] Verify sandbox_cleanup values reflect actual plan terminal state. + [Tags] plan_apply json_envelope sandbox_cleanup + ${result}= Run Process ${PYTHON} ${HELPER} check-sandbox-cleanup cwd=${WORKSPACE} timeout=120s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} plan-apply-json-sandbox-cleanup-ok + +Plan Apply JSON Command Isolation From Other Commands + [Documentation] Verify plan status and cancel do not emit "plan apply" command field. + [Tags] plan_apply json_envelope command_isolation + ${result}= Run Process ${PYTHON} ${HELPER} check-command-isolation cwd=${WORKSPACE} timeout=120s on_timeout=kill + Log ${result.stdout} + Log ${result.stderr} + Should Be Equal As Integers ${result.rc} 0 + Should Contain ${result.stdout} plan-apply-json-command-isolation-ok diff --git a/src/cleveragents/cli/commands/plan.py b/src/cleveragents/cli/commands/plan.py index 19deea26e..ea0ad40f8 100644 --- a/src/cleveragents/cli/commands/plan.py +++ b/src/cleveragents/cli/commands/plan.py @@ -53,6 +53,7 @@ from cleveragents.domain.models.core.error_recovery import ( ErrorCategory, classify_error, ) +from cleveragents.domain.models.core.plan import Plan as LifecyclePlan from cleveragents.domain.models.core.plan import PlanPhase, ProcessingState from cleveragents.infrastructure.sandbox.git_worktree import ( GitWorktreeSandbox, @@ -237,9 +238,6 @@ def _plan_spec_dict(plan: Any) -> dict[str, object]: from cleveragents.domain.models.core.plan import ( ExecutionEnvPriority, ) - from cleveragents.domain.models.core.plan import ( - Plan as LifecyclePlan, - ) if isinstance(plan, LifecyclePlan): result: dict[str, object] = { @@ -346,8 +344,6 @@ def _execute_output_dict( 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 { @@ -477,6 +473,183 @@ 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": "01JAAAAAAAAAAAAAAAAAAAAAAA", + "artifacts": , + "changes": {"insertions": 0, "deletions": 0}, + "project": "", + "applied_at": "", + "validation": { + "tests": {"status": "passed", "passed": N, "total": N}, + "lint": {"status": "passed", "warnings": N}, + "type_check": {"status": "passed", "errors": N}, + "duration_s": 0.0 + }, + "sandbox_cleanup": {"worktree": ..., "branch": ..., + "checkpoint": ...}, + "lifecycle": {"phase": ..., "state": ..., + "total_duration": ..., "total_cost": ..., + "decisions_made": ..., "child_plans": 0} + }, + "timing": {"started": "", + "duration_ms": }, + "messages": ["Changes applied"] + } + + Args: + plan: The ``Plan`` domain model after apply. + + Returns: + A JSON-serialisable dict matching the spec-required envelope. + """ + 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": ["Changes applied"], + } + + # ── Artifacts ───────────────────────────────────────────────────────────── + # Count sandbox refs as a proxy for files changed. + artifacts: int = len(plan.sandbox_refs) + + # ── Changes ─────────────────────────────────────────────────────────────── + # Insertions/deletions are not tracked on the Plan model directly; + # report zeros as a safe default (actual diff data lives in the changeset). + changes: dict[str, object] = {"insertions": 0, "deletions": 0} + + # ── Project ─────────────────────────────────────────────────────────────── + project: str = ( + plan.project_links[0].project_name if plan.project_links else "unknown" + ) + + # ── Applied at ──────────────────────────────────────────────────────────── + applied_at_ts = plan.timestamps.applied_at or plan.timestamps.updated_at + applied_at: str = applied_at_ts.isoformat() + + # ── Validation ──────────────────────────────────────────────────────────── + # Derive from validation_summary when available; fall back to passed values. + validation_summary = plan.validation_summary or {} + + tests_summary = validation_summary.get("tests") or {} + lint_summary = validation_summary.get("lint") or {} + type_check_summary = validation_summary.get("type_check") or {} + + test_passed_all = ( + tests_summary.get("passed", 0) == tests_summary.get("total", 0) + if tests_summary.get("total", 0) > 0 + else True + ) + + validation: dict[str, object] = { + "tests": { + "status": "passed" if test_passed_all else "failed", + "passed": tests_summary.get("passed", 0), + "total": tests_summary.get("total", 0), + }, + "lint": { + "status": "passed" if lint_summary.get("lint_passed", True) else "failed", + "warnings": lint_summary.get("warnings", 0), + }, + "type_check": { + "status": "passed" if type_check_summary.get("type_check_passed", True) else "failed", + "errors": type_check_summary.get("errors", 0), + }, + "duration_s": float( + validation_summary.get("duration_s") or validation_summary.get("duration", 0) + ), + } + + # ── Sandbox cleanup ─────────────────────────────────────────────────────── + # Derive from actual plan terminal state. + is_terminal_apply = ( + plan.is_terminal and plan.phase == PlanPhase.APPLY + ) + sandbox_cleanup: dict[str, object] = { + "worktree": "removed" if is_terminal_apply else "active", + "branch": "merged to main" if is_terminal_apply else "open", + "checkpoint": "archived" if is_terminal_apply else "pending", + } + + # ── Lifecycle ───────────────────────────────────────────────────────────── + # Calculate total_duration from timestamps. + start_ts = ( + plan.timestamps.strategize_started_at or plan.timestamps.created_at + ) + end_ts = plan.timestamps.applied_at or plan.timestamps.updated_at + duration_seconds = int((end_ts - start_ts).total_seconds()) + hours, remainder = divmod(duration_seconds, 3600) + minutes, seconds = divmod(remainder, 60) + total_duration = f"{hours:02d}:{minutes:02d}:{seconds:02d}" + + # Calculate total_cost from cost_metadata when available. + total_cost: str + if plan.cost_metadata is not None: + total_cost = f"${plan.cost_metadata.total_cost:.2f}" + else: + total_cost = "$0.00" + + # Count decisions of type "subplan_spawn" for child_plans. + child_plans: int = sum( + 1 + for d in plan.decisions + if d.get("decision_type") == "subplan_spawn" + ) + + lifecycle: dict[str, object] = { + "phase": plan.phase.value, + "state": plan.processing_state.value, + "total_duration": total_duration, + "total_cost": total_cost, + "decisions_made": len(plan.decisions), + "child_plans": child_plans, + } + + # ── Timing ──────────────────────────────────────────────────────────────── + duration_ms = int((end_ts - start_ts).total_seconds() * 1000) + timing: dict[str, object] = { + "started": start_ts.isoformat(), + "duration_ms": duration_ms, + } + + # ── Data payload ────────────────────────────────────────────────────────── + data: dict[str, object] = { + "plan_id": plan.identity.plan_id, + "artifacts": artifacts, + "changes": changes, + "project": project, + "applied_at": applied_at, + "validation": validation, + "sandbox_cleanup": sandbox_cleanup, + "lifecycle": lifecycle, + } + + return { + "command": "plan apply", + "status": "ok", + "exit_code": 0, + "data": data, + "timing": timing, + "messages": ["Changes applied"], + } + + def _get_current_project() -> Project: """Get the current project or exit with error. @@ -2364,8 +2537,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)