features/executor_error_details.feature

@@ -0,0 +1,34 @@
+@executor-error-details
+Feature: Executor preserves strategy_decisions_json and reports actual actor mode (#10874)
+  Verifies that _run_execute_with_actor merges error_details instead of
+  replacing them, preserving strategy_decisions_json stored by
+  run_strategize.  Also verifies the mode field reflects the actual
+  execute actor type rather than a hardcoded "stub" string.
+
+  Scenario: Execute success preserves strategy_decisions_json in error_details for eed
+    Given a eed mock lifecycle service with strategy_decisions_json in error_details
+    And a eed PlanExecutor with a succeeding execute actor
+    When I eed run execute on the plan
+    Then the eed committed error_details should contain strategy_decisions_json
+    And the eed committed error_details should contain tool_calls_count
+    And the eed committed error_details should contain sandbox_refs_count
+
+  Scenario: Execute success reports actual actor type in mode for eed
+    Given a eed mock lifecycle service with strategy_decisions_json in error_details
+    And a eed PlanExecutor with a succeeding execute actor
+    When I eed run execute on the plan
+    Then the eed committed error_details mode should be the execute actor class name
+
+  Scenario: Execute failure preserves strategy_decisions_json in error_details for eed
+    Given a eed mock lifecycle service with strategy_decisions_json in error_details
+    And a eed PlanExecutor with a failing execute actor
+    When I eed run execute expecting failure on the plan
+    Then the eed committed error_details should contain strategy_decisions_json
+    And the eed committed error_details should contain exception_type
+    And the eed committed error_details should contain traceback
+
+  Scenario: Execute failure reports actual actor type in mode for eed
+    Given a eed mock lifecycle service with strategy_decisions_json in error_details
+    And a eed PlanExecutor with a failing execute actor
+    When I eed run execute expecting failure on the plan
+    Then the eed committed error_details mode should be the execute actor class name

features/plan_executor_coverage.feature

@@ -330,10 +330,10 @@ Feature: PlanExecutor comprehensive coverage
     And the cov2 lifecycle should have called fail_execute
 
   # ------------------------------------------------------------------
-  # PlanExecutor._run_execute_with_stub - error recovery / retry
+  # PlanExecutor._run_execute_with_actor - error recovery / retry
   # ------------------------------------------------------------------
 
-  Scenario: run_execute stub calls fail_execute on exception without recovery
+  Scenario: run_execute actor calls fail_execute on exception without recovery
     Given a cov2 mock lifecycle service
     And a cov2 plan in Execute-Queued state with decision root and definition
     And a cov2 PlanExecutor with failing execute actor and no recovery
@@ -341,7 +341,7 @@ Feature: PlanExecutor comprehensive coverage
     Then a cov2 RuntimeError should have been raised
     And the cov2 lifecycle should have called fail_execute
 
-  Scenario: run_execute stub retries once with error recovery then succeeds
+  Scenario: run_execute actor retries once with error recovery then succeeds
     Given a cov2 mock lifecycle service
     And a cov2 plan in Execute-Queued state with decision root and definition
     And a cov2 PlanExecutor with execute actor that fails once then succeeds
@@ -349,7 +349,7 @@ Feature: PlanExecutor comprehensive coverage
     Then the cov2 execute run result should be an ExecuteResult
     And the cov2 error recovery should have recorded an execute error
 
-  Scenario: run_execute stub exhausts retries with error recovery and fails
+  Scenario: run_execute actor exhausts retries with error recovery and fails
     Given a cov2 mock lifecycle service
     And a cov2 plan in Execute-Queued state with decision root and definition
     And a cov2 PlanExecutor with always-failing execute actor and exhausted retries
@@ -358,7 +358,7 @@ Feature: PlanExecutor comprehensive coverage
     And the cov2 lifecycle should have called fail_execute
     And the cov2 error recovery should have recorded an execute error
 
-  Scenario: run_execute stub with error recovery records error and no retry
+  Scenario: run_execute actor with error recovery records error and no retry
     Given a cov2 mock lifecycle service
     And a cov2 plan in Execute-Queued state with decision root and definition
     And a cov2 PlanExecutor with failing execute actor and recovery denying retry

features/steps/executor_error_details_steps.py

@@ -0,0 +1,240 @@
+"""Steps for executor_error_details.feature (#10874).
+
+Verifies that PlanExecutor._run_execute_with_actor:
+  1. Merges error_details instead of replacing (preserves strategy_decisions_json).
+  2. Reports the actual execute actor class name in the ``mode`` field.
+"""
+
+from __future__ import annotations
+
+import json
+from typing import Any
+from unittest.mock import MagicMock
+
+from behave import given, then, when
+from behave.runner import Context
+
+from cleveragents.application.services.plan_executor import (
+    ExecuteResult,
+    ExecuteStubActor,
+    PlanExecutor,
+)
+from cleveragents.domain.models.core.plan import (
+    PlanPhase,
+    PlanTimestamps,
+    ProcessingState,
+)
+from cleveragents.tool.builtins.changeset import ChangeSet
+
+# ----------------------------------------------------------------------
+# Constants
+# ----------------------------------------------------------------------
+
+EED_PLAN_ID = "01KEEDPLANID000000000PLAN"
+EED_ROOT_ID = "01KEEDROOTID000000000ROOT"
+
+_STRATEGY_DECISIONS = [
+    {
+        "decision_id": "01KEEDDEC0000000000000DEC0",
+        "step_text": "Refactor module A",
+        "sequence": 0,
+        "parent_id": None,
+    },
+    {
+        "decision_id": "01KEEDDEC0000000000000DEC1",
+        "step_text": "Add tests for module A",
+        "sequence": 1,
+        "parent_id": "01KEEDDEC0000000000000DEC0",
+    },
+]
+
+
+# ----------------------------------------------------------------------
+# Helpers
+# ----------------------------------------------------------------------
+
+
+def _eed_make_plan(
+    *,
+    phase: PlanPhase = PlanPhase.EXECUTE,
+    state: ProcessingState = ProcessingState.QUEUED,
+    error_details: dict[str, Any] | None = None,
+) -> MagicMock:
+    """Build a mock plan with strategy_decisions_json in error_details."""
+    plan = MagicMock()
+    plan.phase = phase
+    plan.state = state
+    plan.definition_of_done = "Refactor module A\nAdd tests for module A"
+    plan.decision_root_id = EED_ROOT_ID
+    plan.invariants = []
+    plan.timestamps = PlanTimestamps()
+    plan.changeset_id = None
+    plan.sandbox_refs = []
+    plan.error_details = error_details
+    plan.read_only = False
+    plan.identity.plan_id = EED_PLAN_ID
+    return plan
+
+
+def _eed_make_lifecycle(plan: MagicMock) -> MagicMock:
+    """Build a mock lifecycle service that returns the given plan."""
+    lcs = MagicMock()
+    lcs.get_plan.return_value = plan
+    lcs.start_execute = MagicMock()
+    lcs.complete_execute = MagicMock()
+    lcs.fail_execute = MagicMock()
+    lcs._commit_plan = MagicMock()
+    return lcs
+
+
+# ----------------------------------------------------------------------
+# Given steps
+# ----------------------------------------------------------------------
+
+
+@given("a eed mock lifecycle service with strategy_decisions_json in error_details")
+def step_eed_mock_lifecycle(context: Context) -> None:
+    """Set up a plan in EXECUTE/QUEUED with strategy_decisions_json preserved."""
+    decisions_json = json.dumps(_STRATEGY_DECISIONS)
+    error_details = {
+        "strategy_decisions": "2",
+        "strategy_decisions_json": decisions_json,
+        "invariant_records": "0",
+    }
+    plan = _eed_make_plan(error_details=error_details)
+    lifecycle = _eed_make_lifecycle(plan)
+    context.eed_plan = plan
+    context.eed_lifecycle = lifecycle
+    context.eed_plan_id = EED_PLAN_ID
+
+
+@given("a eed PlanExecutor with a succeeding execute actor")
+def step_eed_executor_succeeding(context: Context) -> None:
+    """Create a PlanExecutor whose execute actor returns a valid result."""
+    context.eed_executor = PlanExecutor(
+        lifecycle_service=context.eed_lifecycle,
+        execution_context=None,
+        error_recovery_service=None,
+    )
+    mock_actor = MagicMock(spec=ExecuteStubActor)
+    mock_actor.execute.return_value = ExecuteResult(
+        changeset_id="cs-001",
+        changeset=ChangeSet(plan_id=EED_PLAN_ID, entries=[]),
+        tool_calls_count=3,
+        sandbox_refs=[],
+    )
+    context.eed_execute_actor = mock_actor
+    context.eed_executor._execute_actor = mock_actor
+
+
+@given("a eed PlanExecutor with a failing execute actor")
+def step_eed_executor_failing(context: Context) -> None:
+    """Create a PlanExecutor whose execute actor raises on execute."""
+    context.eed_executor = PlanExecutor(
+        lifecycle_service=context.eed_lifecycle,
+        execution_context=None,
+        error_recovery_service=None,
+    )
+    mock_actor = MagicMock(spec=ExecuteStubActor)
+    mock_actor.__class__.__name__ = "FakeExecuteActor"
+    mock_actor.execute.side_effect = RuntimeError("eed simulated failure")
+    context.eed_execute_actor = mock_actor
+    context.eed_executor._execute_actor = mock_actor
+
+
+# ----------------------------------------------------------------------
+# When steps
+# ----------------------------------------------------------------------
+
+
+@when("I eed run execute on the plan")
+def step_eed_run_execute(context: Context) -> None:
+    """Run execute and capture the result."""
+    context.eed_result = context.eed_executor.run_execute(context.eed_plan_id)
+    # Capture the error_details that were committed
+    commit_calls = context.eed_lifecycle._commit_plan.call_args_list
+    if commit_calls:
+        last_plan = commit_calls[-1][0][0]
+        context.eed_committed_error_details = last_plan.error_details
+    else:
+        context.eed_committed_error_details = {}
+
+
+@when("I eed run execute expecting failure on the plan")
+def step_eed_run_execute_failure(context: Context) -> None:
+    """Run execute expecting it to raise, capture the committed error_details."""
+    try:
+        context.eed_executor.run_execute(context.eed_plan_id)
+        context.eed_raised = False
+    except Exception:
+        context.eed_raised = True
+    # Capture the error_details that were committed
+    commit_calls = context.eed_lifecycle._commit_plan.call_args_list
+    if commit_calls:
+        last_plan = commit_calls[-1][0][0]
+        context.eed_committed_error_details = last_plan.error_details
+    else:
+        context.eed_committed_error_details = {}
+
+
+# ----------------------------------------------------------------------
+# Then steps
+# ----------------------------------------------------------------------
+
+
+@then("the eed committed error_details should contain strategy_decisions_json")
+def step_eed_check_strategy_json(context: Context) -> None:
+    """Verify strategy_decisions_json was NOT destroyed by execute."""
+    details = context.eed_committed_error_details
+    assert "strategy_decisions_json" in details, (
+        f"strategy_decisions_json missing from error_details: {details.keys()}"
+    )
+    parsed = json.loads(details["strategy_decisions_json"])
+    assert len(parsed) == 2, f"Expected 2 strategy decisions, got {len(parsed)}"
+    assert parsed[0]["step_text"] == "Refactor module A"
+
+
+@then("the eed committed error_details should contain tool_calls_count")
+def step_eed_check_tool_calls(context: Context) -> None:
+    details = context.eed_committed_error_details
+    assert "tool_calls_count" in details, (
+        f"tool_calls_count missing from error_details: {details.keys()}"
+    )
+
+
+@then("the eed committed error_details should contain sandbox_refs_count")
+def step_eed_check_sandbox_refs(context: Context) -> None:
+    details = context.eed_committed_error_details
+    assert "sandbox_refs_count" in details, (
+        f"sandbox_refs_count missing from error_details: {details.keys()}"
+    )
+
+
+@then("the eed committed error_details should contain exception_type")
+def step_eed_check_exception_type(context: Context) -> None:
+    details = context.eed_committed_error_details
+    assert "exception_type" in details, (
+        f"exception_type missing from error_details: {details.keys()}"
+    )
+    assert details["exception_type"] == "RuntimeError"
+
+
+@then("the eed committed error_details should contain traceback")
+def step_eed_check_traceback(context: Context) -> None:
+    details = context.eed_committed_error_details
+    assert "traceback" in details, (
+        f"traceback missing from error_details: {details.keys()}"
+    )
+
+
+@then("the eed committed error_details mode should be the execute actor class name")
+def step_eed_check_mode(context: Context) -> None:
+    """Verify mode reflects the actual actor type, not hardcoded 'stub'."""
+    details = context.eed_committed_error_details
+    assert "mode" in details, f"mode missing from error_details: {details.keys()}"
+    actual_mode = details["mode"]
+    assert actual_mode != "stub", (
+        "mode is still hardcoded as 'stub', expected actor class name"
+    )
+    expected = type(context.eed_execute_actor).__name__
+    assert actual_mode == expected, f"mode is '{actual_mode}', expected '{expected}'"

features/steps/plan_executor_coverage_steps.py

@@ -903,7 +903,7 @@ def step_cov2_check_fail_execute(context: Context) -> None:
 
 
 # ----------------------------------------------------------------------
-# PlanExecutor._run_execute_with_stub - retry logic
+# PlanExecutor._run_execute_with_actor - retry logic
 # ----------------------------------------------------------------------
 
 

src/cleveragents/application/services/plan_executor.py

@@ -821,7 +821,7 @@ class PlanExecutor:
             raise ValidationError("plan_id must not be empty")
         if self._execution_context is not None:
             return self._run_execute_with_runtime(plan_id, stream_callback)
-        return self._run_execute_with_stub(plan_id, stream_callback)
+        return self._run_execute_with_actor(plan_id, stream_callback)
 
     def _build_decisions(self, plan: Any) -> list[StrategyDecision]:
         """Build decisions for the Execute phase.
@@ -843,7 +843,7 @@ class PlanExecutor:
             try:
                 raw_list: list[dict[str, Any]] = json.loads(stored_json)
                 return [StrategyDecision.model_validate(d) for d in raw_list]
-            except Exception:
+            except (json.JSONDecodeError, ValidationError):
                 self._logger.warning(
                     "Failed to deserialise stored strategy decisions; "
                     "falling back to definition_of_done parsing",
@@ -953,12 +953,16 @@ class PlanExecutor:
             plan = self._lifecycle.get_plan(plan_id)
             plan.changeset_id = result.changeset_id
             plan.sandbox_refs = result.sandbox_refs
-            plan.error_details = {
-                "tool_call_count": str(result.tool_call_count),
-                "decisions_processed": str(len(result.decision_ids_processed)),
-                "execution_duration_ms": str(result.execution_duration_ms),
-                "mode": "runtime",
-            }
+            existing = dict(plan.error_details or {})
+            existing.update(
+                {
+                    "tool_call_count": str(result.tool_call_count),
+                    "decisions_processed": str(len(result.decision_ids_processed)),
+                    "execution_duration_ms": str(result.execution_duration_ms),
+                    "mode": "runtime",
+                }
+            )
+            plan.error_details = existing
             plan.timestamps.updated_at = datetime.now(tz=UTC)
 
             # Spawn and execute child subplans from spawn decisions
@@ -996,21 +1000,30 @@ class PlanExecutor:
             self._try_rollback_to_last_checkpoint(plan_id)
             error_msg = f"{type(exc).__name__}: {exc}"
             plan = self._lifecycle.get_plan(plan_id)
-            plan.error_details = {
-                "exception_type": type(exc).__name__,
-                "traceback": traceback.format_exc(),
-                "mode": "runtime",
-            }
+            existing = dict(plan.error_details or {})
+            existing.update(
+                {
+                    "exception_type": type(exc).__name__,
+                    "traceback": traceback.format_exc(),
+                    "mode": "runtime",
+                }
+            )
+            plan.error_details = existing
             self._lifecycle._commit_plan(plan)
             self._lifecycle.fail_execute(plan_id, error_msg)
             raise
 
-    def _run_execute_with_stub(
+    def _run_execute_with_actor(
         self,
         plan_id: str,
         stream_callback: StreamCallback | None = None,
     ) -> ExecuteResult:
-        """Execute using the legacy ExecuteStubActor with optional retry."""
+        """Execute using the configured execute actor with optional retry.
+
+        Retry behaviour is controlled by the ``ErrorRecoveryService``
+        attached at construction time (``error_recovery.max_retries``).
+        Without an error recovery service, failures are immediate.
+        """
         plan = self._guard_execute(plan_id)
         decisions = self._build_decisions(plan)
 
@@ -1043,11 +1056,15 @@ class PlanExecutor:
                 plan = self._lifecycle.get_plan(plan_id)
                 plan.changeset_id = result.changeset_id
                 plan.sandbox_refs = result.sandbox_refs
-                plan.error_details = {
-                    "tool_calls_count": str(result.tool_calls_count),
-                    "sandbox_refs_count": str(len(result.sandbox_refs)),
-                    "mode": "stub",
-                }
+                existing = dict(plan.error_details or {})
+                existing.update(
+                    {
+                        "tool_calls_count": str(result.tool_calls_count),
+                        "sandbox_refs_count": str(len(result.sandbox_refs)),
+                        "mode": type(self._execute_actor).__name__,
+                    }
+                )
+                plan.error_details = existing
                 plan.timestamps.updated_at = datetime.now(tz=UTC)
 
                 # Spawn and execute child subplans from spawn decisions
@@ -1067,8 +1084,9 @@ class PlanExecutor:
                     OperationalMetricKey.PLAN_DURATION_MS, plan_id, _duration_ms
                 )
                 self._logger.info(
-                    "Execute completed (stub)",
+                    "Execute completed",
                     plan_id=plan_id,
+                    mode=type(self._execute_actor).__name__,
                     changeset_id=result.changeset_id,
                     tool_calls=result.tool_calls_count,
                 )
@@ -1110,17 +1128,21 @@ class PlanExecutor:
                 "on_error",
                 {
                     "exception_type": type(last_exc).__name__,
-                    "mode": "stub",
+                    "mode": type(self._execute_actor).__name__,
                 },
             )
         self._try_rollback_to_last_checkpoint(plan_id)
         error_msg = f"{type(last_exc).__name__}: {last_exc}"
         plan = self._lifecycle.get_plan(plan_id)
-        plan.error_details = {
-            "exception_type": type(last_exc).__name__,
-            "traceback": traceback.format_exc(),
-            "mode": "stub",
-        }
+        existing = dict(plan.error_details or {})
+        existing.update(
+            {
+                "exception_type": type(last_exc).__name__,
+                "traceback": traceback.format_exc(),
+                "mode": type(self._execute_actor).__name__,
+            }
+        )
+        plan.error_details = existing
         self._lifecycle._commit_plan(plan)
         self._lifecycle.fail_execute(plan_id, error_msg)
         raise last_exc