fix(cli): resolve unit_tests failures in error_handling and sandbox concurrency

- Use @step instead of @given for debug mode steps in cli_error_handling_steps.py
  so they match regardless of inherited keyword type (Given/When/Then) per
  Behave's type-specific step registry

- Extend commit_all's _lock scope to cover the commit phase in SandboxManager
  so concurrent get_or_create_sandbox calls are blocked while commits run,
  satisfying the sandbox_manager_concurrency feature expectation

features/steps/cli_error_handling_steps.py

@@ -6,7 +6,7 @@ import io
 import sys
 from typing import Any
 
-from behave import given, then, when
+from behave import given, step, then, when
 
 from cleveragents.cli.output import CLIOutputManager, display_success, display_warning
 from cleveragents.core.exceptions import CleverAgentsError
@@ -73,13 +73,13 @@ def step_display_error(context: Any, label: str, message: str) -> None:
         sys.stderr = old_stderr
 
 
-@given("debug mode is disabled")
+@step("debug mode is disabled")
 def step_disable_debug(context: Any) -> None:
     """Disable debug mode."""
     context.output_manager.debug = False
 
 
-@given("debug mode is enabled")
+@step("debug mode is enabled")
 def step_enable_debug(context: Any) -> None:
     """Enable debug mode."""
     context.output_manager.debug = True

src/cleveragents/infrastructure/sandbox/manager.py

@@ -246,13 +246,11 @@ class SandboxManager:
 
         .. warning:: Thread safety
 
-           This method is **not safe** for concurrent calls on the same
-           *plan_id*.  The internal lock serializes access to the
-           sandbox registry, but individual sandbox ``commit()`` and
-           ``rollback()`` calls run outside the lock to avoid blocking
-           all manager operations during potentially slow I/O.
-           Callers must ensure that ``commit_all`` is not invoked
-           concurrently for the same plan.
+           The internal lock is held for the duration of the commit
+           phase, which blocks concurrent ``get_or_create_sandbox``
+           calls until all commits complete.  Concurrent calls to
+           ``commit_all`` for the same *plan_id* must still be avoided
+           by the caller.
         """
         if not plan_id:
             raise ValueError("plan_id cannot be empty")
@@ -260,101 +258,101 @@ class SandboxManager:
         with self._lock:
             sandboxes = list(self._active_sandboxes.get(plan_id, {}).values())
 
-        committable = [
-            sb
-            for sb in sandboxes
-            if sb.status in (SandboxStatus.CREATED, SandboxStatus.ACTIVE)
-        ]
+            committable = [
+                sb
+                for sb in sandboxes
+                if sb.status in (SandboxStatus.CREATED, SandboxStatus.ACTIVE)
+            ]
 
-        if not committable:
-            return []
+            if not committable:
+                return []
 
-        # Warn about sandboxes that cannot be rolled back (e.g. NoSandbox,
-        # TransactionSandbox after COMMIT).  These break the all-or-nothing
-        # guarantee if a later commit in the batch fails.
-        non_rollbackable: list[Sandbox] = []
-        rollbackable: list[Sandbox] = []
-        for sb in committable:
-            if isinstance(sb, NoSandbox):
-                logger.warning(
-                    "Sandbox %s (resource strategy 'none') cannot be rolled "
-                    "back after commit.  Atomicity is broken for plan %s "
-                    "because changes are applied in-place immediately.",
-                    sb.sandbox_id,
-                    plan_id,
-                )
-                non_rollbackable.append(sb)
-            elif isinstance(sb, TransactionSandbox):
-                logger.warning(
-                    "Sandbox %s (transaction_rollback strategy) cannot be "
-                    "rolled back after database COMMIT.  Atomicity is "
-                    "broken for plan %s because database changes are "
-                    "permanent once committed.",
-                    sb.sandbox_id,
-                    plan_id,
-                )
-                non_rollbackable.append(sb)
-            else:
-                rollbackable.append(sb)
+            # Warn about sandboxes that cannot be rolled back (e.g. NoSandbox,
+            # TransactionSandbox after COMMIT).  These break the all-or-nothing
+            # guarantee if a later commit in the batch fails.
+            non_rollbackable: list[Sandbox] = []
+            rollbackable: list[Sandbox] = []
+            for sb in committable:
+                if isinstance(sb, NoSandbox):
+                    logger.warning(
+                        "Sandbox %s (resource strategy 'none') cannot be rolled "
+                        "back after commit.  Atomicity is broken for plan %s "
+                        "because changes are applied in-place immediately.",
+                        sb.sandbox_id,
+                        plan_id,
+                    )
+                    non_rollbackable.append(sb)
+                elif isinstance(sb, TransactionSandbox):
+                    logger.warning(
+                        "Sandbox %s (transaction_rollback strategy) cannot be "
+                        "rolled back after database COMMIT.  Atomicity is "
+                        "broken for plan %s because database changes are "
+                        "permanent once committed.",
+                        sb.sandbox_id,
+                        plan_id,
+                    )
+                    non_rollbackable.append(sb)
+                else:
+                    rollbackable.append(sb)
 
-        # Commit rollbackable sandboxes first so that if any fail, we
-        # can undo them.  Non-rollbackable sandboxes commit last — they
-        # only run after all rollbackable sandboxes succeed.
-        ordered = rollbackable + non_rollbackable
+            # Commit rollbackable sandboxes first so that if any fail, we
+            # can undo them.  Non-rollbackable sandboxes commit last — they
+            # only run after all rollbackable sandboxes succeed.
+            ordered = rollbackable + non_rollbackable
 
-        committed: list[tuple[Sandbox, CommitResult]] = []
+            committed: list[tuple[Sandbox, CommitResult]] = []
 
-        for sandbox in ordered:
-            try:
-                result = sandbox.commit()
-                committed.append((sandbox, result))
-            except Exception as exc:
-                # Atomic rollback: undo all previously-committed sandboxes.
-                # Catches Exception (not just SandboxError) so that
-                # unexpected errors cannot bypass the rollback and leave
-                # already-committed sandboxes in an inconsistent state.
-                failed_id = sandbox.sandbox_id
-                rolled_back_ids, failed_rollback_ids = self._rollback_committed(
-                    committed, plan_id
-                )
-
-                error_msg = f"Atomic commit failed at sandbox {failed_id}: {exc}"
-                if rolled_back_ids:
-                    error_msg += f"; rolled back sandboxes: {rolled_back_ids}"
-                if failed_rollback_ids:
-                    error_msg += (
-                        f"; FAILED to roll back sandboxes: {failed_rollback_ids}"
+            for sandbox in ordered:
+                try:
+                    result = sandbox.commit()
+                    committed.append((sandbox, result))
+                except Exception as exc:
+                    # Atomic rollback: undo all previously-committed sandboxes.
+                    # Catches Exception (not just SandboxError) so that
+                    # unexpected errors cannot bypass the rollback and leave
+                    # already-committed sandboxes in an inconsistent state.
+                    failed_id = sandbox.sandbox_id
+                    rolled_back_ids, failed_rollback_ids = self._rollback_committed(
+                        committed, plan_id
                     )
 
-                logger.error(
-                    "Atomic commit_all failed for plan %s: %s",
-                    plan_id,
-                    error_msg,
-                )
+                    error_msg = f"Atomic commit failed at sandbox {failed_id}: {exc}"
+                    if rolled_back_ids:
+                        error_msg += f"; rolled back sandboxes: {rolled_back_ids}"
+                    if failed_rollback_ids:
+                        error_msg += (
+                            f"; FAILED to roll back sandboxes: {failed_rollback_ids}"
+                        )
 
-                # For non-SandboxError exceptions, wrap in
-                # AtomicCommitError carrying rollback metadata so the
-                # caller can determine which sandboxes were rolled back.
-                # The original exception is chained as __cause__.
-                if not isinstance(exc, SandboxError):
-                    raise AtomicCommitError(
+                    logger.error(
+                        "Atomic commit_all failed for plan %s: %s",
+                        plan_id,
                         error_msg,
-                        rolled_back_ids=rolled_back_ids,
-                        failed_rollback_ids=failed_rollback_ids,
-                    ) from exc
-
-                return [
-                    CommitResult(
-                        sandbox_id=failed_id,
-                        success=False,
-                        error=error_msg,
-                        timestamp=datetime.now(),
-                        metadata={
-                            "rolled_back": rolled_back_ids,
-                            "rollback_failed": failed_rollback_ids,
-                        },
                     )
-                ]
+
+                    # For non-SandboxError exceptions, wrap in
+                    # AtomicCommitError carrying rollback metadata so the
+                    # caller can determine which sandboxes were rolled back.
+                    # The original exception is chained as __cause__.
+                    if not isinstance(exc, SandboxError):
+                        raise AtomicCommitError(
+                            error_msg,
+                            rolled_back_ids=rolled_back_ids,
+                            failed_rollback_ids=failed_rollback_ids,
+                        ) from exc
+
+                    return [
+                        CommitResult(
+                            sandbox_id=failed_id,
+                            success=False,
+                            error=error_msg,
+                            timestamp=datetime.now(),
+                            metadata={
+                                "rolled_back": rolled_back_ids,
+                                "rollback_failed": failed_rollback_ids,
+                            },
+                        )
+                    ]
 
         return [result for _, result in committed]