CHANGELOG.md

@@ -221,6 +221,20 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
   `pr-merge-pool-supervisor` to the product-builder's supervisor launch list (18 total
   supervisors). Updated all numeric references, pre-flight checklists, and validation logic.
 
+- `ActionRepository.update()` now uses explicit bulk `sa_delete()` + `session.flush()`
+  before re-inserting child rows for `action_arguments` and `action_invariants`, fixing
+  a `sqlite3.IntegrityError: UNIQUE constraint failed` crash when `agents plan use` was
+  called on an action that already had arguments registered via `action create`. (#4197)
+
+- Fixed CheckpointManager not wired into PlanExecutor — checkpoints were
+  never created during plan execution because `_get_plan_executor()` in
+  the CLI constructed PlanExecutor without a CheckpointManager (defaulted
+  to None, silently skipping all checkpoint hooks). `_get_plan_executor()`
+  now resolves the container singleton so CLI `plan execute` and `plan
+  rollback` share the same registry, and `_try_create_checkpoint()` raises
+  `PlanError` if checkpoint metadata cannot be persisted. Writable sandboxable
+  resources and write-capable tools now default to `checkpointable=True`, and
+  new Behave scenarios cover DI wiring, rollback, and capability defaults. (#1253)
 ---
 
 ## [3.8.0] — 2026-04-05

features/checkpoint_wiring.feature

@@ -0,0 +1,63 @@
+@checkpoint-wiring
+Feature: CheckpointManager wired into PlanExecutor (#1253)
+  Verifies that checkpoints are automatically created during
+  plan execution and that last_checkpoint_id is set on the plan.
+
+  Scenario: PlanExecutor receives CheckpointManager from CLI factory for ckpt_wire
+    When I build a PlanExecutor via _get_plan_executor for ckpt_wire
+    Then the executor checkpoint_manager should not be None for ckpt_wire
+
+  Scenario: PlanExecutor reuses CheckpointManager singleton from DI container for ckpt_wire
+    When I build a PlanExecutor via _get_plan_executor for ckpt_wire
+    And I get CheckpointManager from the DI container for ckpt_wire
+    Then the executor checkpoint_manager should be the container singleton for ckpt_wire
+
+  Scenario: CheckpointManager is registered in DI container for ckpt_wire
+    When I get CheckpointManager from the DI container for ckpt_wire
+    Then the container checkpoint_manager should not be None for ckpt_wire
+
+  Scenario: _try_create_checkpoint creates real checkpoint when manager present for ckpt_wire
+    Given a PlanExecutor with a real CheckpointManager for ckpt_wire
+    And a plan exists in the lifecycle service for ckpt_wire
+    And a sandbox exists for the test plan for ckpt_wire
+    When I call _try_create_checkpoint for ckpt_wire
+    Then a SandboxCheckpoint should be returned for ckpt_wire
+
+  Scenario: _try_create_checkpoint returns None when manager is None for ckpt_wire
+    Given a PlanExecutor with no CheckpointManager for ckpt_wire
+    When I call _try_create_checkpoint for ckpt_wire
+    Then None should be returned for ckpt_wire
+
+  Scenario: Checkpoint creation sets last_checkpoint_id on plan for ckpt_wire
+    Given a PlanExecutor with a real CheckpointManager for ckpt_wire
+    And a plan exists in the lifecycle service for ckpt_wire
+    And a sandbox exists for the test plan for ckpt_wire
+    When I call _try_create_checkpoint for ckpt_wire
+    Then the plan last_checkpoint_id should not be None for ckpt_wire
+
+  Scenario: Plan rollback restores sandbox from auto checkpoint for ckpt_wire
+    Given a PlanExecutor with a real CheckpointManager for ckpt_wire
+    And a plan exists in the lifecycle service for ckpt_wire
+    And a sandbox exists for the test plan for ckpt_wire
+    When I call _try_create_checkpoint for ckpt_wire
+    And I mutate the sandbox file for ckpt_wire
+    And I rollback to the last checkpoint via the executor for ckpt_wire
+    Then rollback should succeed for ckpt_wire
+    And the sandbox file should match the original contents for ckpt_wire
+
+  Scenario: CheckpointManager takes no constructor arguments for ckpt_wire
+    When I instantiate CheckpointManager with no args for ckpt_wire
+    Then CheckpointManager should be created successfully for ckpt_wire
+
+  Scenario: Writable tools and resources default to checkpointable for ckpt_wire
+    When I create default capabilities for a writable resource for ckpt_wire
+    And I create default capabilities for a writable tool for ckpt_wire
+    Then the resource capability should be checkpointable for ckpt_wire
+    And the tool capability should be checkpointable for ckpt_wire
+
+  Scenario: Non-writable resource defaults to not checkpointable for ckpt_wire
+    When I create capabilities for a non-writable resource for ckpt_wire
+    Then the resource capability should not be checkpointable for ckpt_wire
+
+  Scenario: Non-writable resource with explicit checkpointable raises error for ckpt_wire
+    Then creating a non-writable resource with checkpointable True should raise ValueError for ckpt_wire

features/consolidated_resource.feature

@@ -418,7 +418,7 @@ Feature: Consolidated Resource
     Then the capability readable should be true
     And the capability writable should be true
     And the capability sandboxable should be true
-    And the capability checkpointable should be false
+    And the capability checkpointable should be true
 
 
   Scenario: ResourceCapabilities with custom values
@@ -463,10 +463,9 @@ Feature: Consolidated Resource
     Then the resource sandbox strategy should be "git_worktree"
 
 
-  Scenario: Resource with capabilities
-    Given a Resource with sandboxable false and checkpointable true
-    Then the resource can_sandbox should be false
-    And the resource can_checkpoint should be true
+  Scenario: Resource with non-sandboxable rejects checkpointable
+    When I try to create a Resource with sandboxable false and checkpointable true
+    Then a resource validation error should be raised
 
     # Resource model - validation
 

features/consolidated_skill.feature

@@ -485,7 +485,7 @@ Feature: Consolidated Skill
     And the flatten summary read_only_tools should be 1
     And the flatten summary write_tools should be 1
     And the flatten summary has_side_effects should be true
-    And the flatten summary checkpointable_tools should be 1
+    And the flatten summary checkpointable_tools should be 2
 
     # ---------------------------------------------------------------------------
     # tools() method returns both entries and summary

features/steps/checkpoint_wiring_steps.py

@@ -0,0 +1,297 @@
+"""Step definitions for checkpoint_wiring.feature."""
+
+from __future__ import annotations
+
+import tempfile
+from pathlib import Path
+
+from behave import given, then, when
+
+from cleveragents.infrastructure.sandbox.checkpoint import (
+    CheckpointManager,
+    SandboxCheckpoint,
+)
+
+# ── CLI factory ──────────────────────────────────────────
+
+
+@when("I build a PlanExecutor via _get_plan_executor for ckpt_wire")
+def step_build_executor(context: object) -> None:
+    from cleveragents.cli.commands.plan import _get_plan_executor
+
+    executor = _get_plan_executor()
+    context.ckpt_executor = executor
+
+
+@then("the executor checkpoint_manager should not be None for ckpt_wire")
+def step_executor_has_mgr(context: object) -> None:
+    executor = context.ckpt_executor
+    assert executor._checkpoint_manager is not None, (
+        "PlanExecutor._checkpoint_manager is None -- not wired"
+    )
+
+
+# ── DI container ─────────────────────────────────────────
+
+
+@when("I get CheckpointManager from the DI container for ckpt_wire")
+def step_container_mgr(context: object) -> None:
+    from cleveragents.application.container import get_container
+
+    container = get_container()
+    context.ckpt_container_mgr = container.checkpoint_manager()
+
+
+@then("the container checkpoint_manager should not be None for ckpt_wire")
+def step_container_has_mgr(context: object) -> None:
+    mgr = context.ckpt_container_mgr
+    assert mgr is not None
+    assert isinstance(mgr, CheckpointManager)
+
+
+@then("the executor checkpoint_manager should be the container singleton for ckpt_wire")
+def step_executor_matches_container(context: object) -> None:
+    executor = context.ckpt_executor
+    container_mgr = context.ckpt_container_mgr
+    assert executor._checkpoint_manager is container_mgr, (
+        "PlanExecutor did not reuse container CheckpointManager singleton"
+    )
+
+
+# ── _try_create_checkpoint with real manager ─────────────
+
+
+@given("a PlanExecutor with a real CheckpointManager for ckpt_wire")
+def step_executor_with_mgr(context: object) -> None:
+    from cleveragents.application.services.plan_executor import PlanExecutor
+    from cleveragents.application.services.plan_lifecycle_service import (
+        PlanLifecycleService,
+    )
+    from cleveragents.config.settings import Settings
+
+    lifecycle = PlanLifecycleService(settings=Settings())
+    mgr = CheckpointManager()
+    executor = PlanExecutor(
+        lifecycle_service=lifecycle,
+        checkpoint_manager=mgr,
+    )
+    context.ckpt_executor = executor
+    context.ckpt_lifecycle = lifecycle
+    context.ckpt_manager = mgr
+    context.ckpt_plan_id = "plan-ckpt-test-01"
+
+
+@given("a sandbox exists for the test plan for ckpt_wire")
+def step_sandbox_exists(context: object) -> None:
+    from cleveragents.infrastructure.sandbox.factory import SandboxFactory
+    from cleveragents.infrastructure.sandbox.manager import SandboxManager
+
+    plan_id = context.ckpt_plan_id
+    tmpdir = tempfile.mkdtemp(prefix="ckpt-wire-")
+    source_file = Path(tmpdir, "test.txt")
+    initial_content = "checkpoint test"
+    source_file.write_text(initial_content)
+
+    factory = SandboxFactory()
+    mgr = SandboxManager(factory=factory, cleanup_on_exit=False)
+    sandbox = mgr.get_or_create_sandbox(
+        plan_id=plan_id,
+        resource_id="res-ckpt-test",
+        original_path=tmpdir,
+        sandbox_strategy="copy_on_write",
+    )
+    context.ckpt_sandbox = sandbox
+
+    sandbox_path = Path(sandbox.context.sandbox_path)
+    sandbox_path.mkdir(parents=True, exist_ok=True)
+    sandbox_file = sandbox_path / "test.txt"
+    if not sandbox_file.exists():
+        sandbox_file.write_text(initial_content)
+    context.ckpt_sandbox_file = sandbox_file
+    context.ckpt_original_content = sandbox_file.read_text()
+
+    # Patch the executor's sandbox resolver to return our sandbox
+    executor = context.ckpt_executor
+    executor._resolve_sandbox_for_checkpoint = lambda pid: sandbox
+
+
+@given("a plan exists in the lifecycle service for ckpt_wire")
+def step_plan_exists(context: object) -> None:
+    from cleveragents.domain.models.core.plan import ProjectLink
+
+    lifecycle = context.ckpt_lifecycle
+    lifecycle.create_action(
+        name="local/ckpt-wire-action",
+        description="Checkpoint wiring test",
+        definition_of_done="Checkpoint created",
+        strategy_actor="local/stub",
+        execution_actor="local/stub",
+    )
+    plan = lifecycle.use_action(
+        action_name="local/ckpt-wire-action",
+        project_links=[ProjectLink(project_name="test")],
+    )
+    context.ckpt_plan_id = plan.identity.plan_id
+
+    # Re-patch resolver with correct plan_id
+    executor = context.ckpt_executor
+    sandbox = getattr(context, "ckpt_sandbox", None)
+    if sandbox is not None:
+        executor._resolve_sandbox_for_checkpoint = lambda pid: sandbox
+
+
+@when("I call _try_create_checkpoint for ckpt_wire")
+def step_try_checkpoint(context: object) -> None:
+    executor = context.ckpt_executor
+    plan_id = context.ckpt_plan_id
+    result = executor._try_create_checkpoint(plan_id, "test_phase")
+    context.ckpt_result = result
+
+
+@then("a SandboxCheckpoint should be returned for ckpt_wire")
+def step_checkpoint_returned(context: object) -> None:
+    result = context.ckpt_result
+    assert result is not None, "Expected SandboxCheckpoint, got None"
+    assert isinstance(result, SandboxCheckpoint)
+
+
+@then("None should be returned for ckpt_wire")
+def step_none_returned(context: object) -> None:
+    result = context.ckpt_result
+    assert result is None
+
+
+@then("the plan last_checkpoint_id should not be None for ckpt_wire")
+def step_plan_has_checkpoint(context: object) -> None:
+    lifecycle = context.ckpt_lifecycle
+    plan_id = context.ckpt_plan_id
+    plan = lifecycle.get_plan(plan_id)
+    assert plan.last_checkpoint_id is not None, (
+        f"plan.last_checkpoint_id is None for plan {plan_id}"
+    )
+
+
+@when("I mutate the sandbox file for ckpt_wire")
+def step_mutate_sandbox(context: object) -> None:
+    file_path = context.ckpt_sandbox_file
+    mutated_content = "checkpoint test mutated"
+    file_path.write_text(mutated_content)
+    context.ckpt_mutated_content = mutated_content
+
+
+@when("I rollback to the last checkpoint via the executor for ckpt_wire")
+def step_rollback_to_checkpoint(context: object) -> None:
+    executor = context.ckpt_executor
+    plan_id = context.ckpt_plan_id
+    success = executor._try_rollback_to_last_checkpoint(plan_id)
+    context.ckpt_rollback_success = success
+
+
+@then("rollback should succeed for ckpt_wire")
+def step_verify_rollback_success(context: object) -> None:
+    assert context.ckpt_rollback_success is True, "Rollback did not succeed"
+
+
+@then("the sandbox file should match the original contents for ckpt_wire")
+def step_verify_sandbox_restored(context: object) -> None:
+    file_path = context.ckpt_sandbox_file
+    assert file_path.read_text() == context.ckpt_original_content, (
+        "Sandbox contents were not restored from checkpoint"
+    )
+
+
+# ── PlanExecutor with no manager ─────────────────────────
+
+
+@given("a PlanExecutor with no CheckpointManager for ckpt_wire")
+def step_executor_no_mgr(context: object) -> None:
+    from cleveragents.application.services.plan_executor import PlanExecutor
+    from cleveragents.application.services.plan_lifecycle_service import (
+        PlanLifecycleService,
+    )
+    from cleveragents.config.settings import Settings
+
+    executor = PlanExecutor(
+        lifecycle_service=PlanLifecycleService(settings=Settings()),
+    )
+    context.ckpt_executor = executor
+    context.ckpt_plan_id = "plan-no-mgr"
+
+
+# ── CheckpointManager no-arg construction ────────────────
+
+
+@when("I instantiate CheckpointManager with no args for ckpt_wire")
+def step_instantiate_mgr(context: object) -> None:
+    context.ckpt_new_mgr = CheckpointManager()
+
+
+@then("CheckpointManager should be created successfully for ckpt_wire")
+def step_mgr_created(context: object) -> None:
+    mgr = context.ckpt_new_mgr
+    assert mgr is not None
+    assert isinstance(mgr, CheckpointManager)
+
+
+# ── Default capability behaviour ─────────────────────────
+
+
+@when("I create default capabilities for a writable resource for ckpt_wire")
+def step_create_resource_capabilities(context: object) -> None:
+    from cleveragents.domain.models.core.resource import ResourceCapabilities
+
+    # Explicit flags to exercise the model_validator that derives
+    # checkpointable from writable AND sandboxable.
+    context.ckpt_resource_caps = ResourceCapabilities(
+        writable=True,
+        sandboxable=True,
+    )
+
+
+@then("the resource capability should be checkpointable for ckpt_wire")
+def step_resource_checkpointable(context: object) -> None:
+    capabilities = context.ckpt_resource_caps
+    assert capabilities.checkpointable is True
+
+
+@when("I create default capabilities for a writable tool for ckpt_wire")
+def step_create_tool_capabilities(context: object) -> None:
+    from cleveragents.domain.models.core.tool import ToolCapability
+
+    context.ckpt_tool_cap = ToolCapability(writes=True)
+
+
+@then("the tool capability should be checkpointable for ckpt_wire")
+def step_tool_checkpointable(context: object) -> None:
+    capability = context.ckpt_tool_cap
+    assert capability.checkpointable is True
+
+
+@when("I create capabilities for a non-writable resource for ckpt_wire")
+def step_create_non_writable_resource(context: object) -> None:
+    from cleveragents.domain.models.core.resource import ResourceCapabilities
+
+    context.ckpt_resource_caps = ResourceCapabilities(writable=False)
+
+
+@then("the resource capability should not be checkpointable for ckpt_wire")
+def step_resource_not_checkpointable(context: object) -> None:
+    capabilities = context.ckpt_resource_caps
+    assert capabilities.checkpointable is False, (
+        f"Expected checkpointable=False for non-writable resource, "
+        f"got {capabilities.checkpointable}"
+    )
+
+
+@then(
+    "creating a non-writable resource with checkpointable True "
+    "should raise ValueError for ckpt_wire"
+)
+def step_non_writable_checkpointable_raises(context: object) -> None:
+    from cleveragents.domain.models.core.resource import ResourceCapabilities
+
+    try:
+        ResourceCapabilities(writable=False, checkpointable=True)
+        raise AssertionError("Expected ValueError but no exception was raised")
+    except ValueError:
+        pass  # Expected

features/steps/resource_registry_model_steps.py

@@ -119,6 +119,16 @@ def step_check_capability(context: Context, field: str, expected: str) -> None:
     assert actual == expected_bool, f"Expected {field}={expected_bool}, got {actual}"
 
 
+@when("I try to create a Resource with sandboxable false and checkpointable true")
+def step_create_resource_non_sandboxable_checkpointable(context: Context) -> None:
+    """Try to create a Resource with conflicting capabilities."""
+    try:
+        ResourceCapabilities(sandboxable=False, checkpointable=True)
+        context.resource_error = None
+    except (ValidationError, ValueError) as e:
+        context.resource_error = e
+
+
 @when("I try to mutate a ResourceCapabilities field")
 def step_try_mutate_capabilities(context: Context) -> None:
     """Try to mutate a frozen ResourceCapabilities."""

robot/helper_skill_flatten.py

@@ -270,7 +270,9 @@ def _capability_summary() -> int:
         summary.total_tools == 3,
         summary.read_only_tools == 1,
         summary.write_tools == 1,
-        summary.checkpointable_tools == 1,
+        # Writer tool now defaults to checkpointable=True (writes=True
+        # AND NOT read_only), plus the explicit Checkpoint tool = 2.
+        summary.checkpointable_tools == 2,
         summary.has_side_effects is True,
     ]
     if not all(checks):

src/cleveragents/application/container.py

@@ -113,6 +113,7 @@ from cleveragents.infrastructure.plugins.extension_catalog import (
     register_all_extension_points,
 )
 from cleveragents.infrastructure.plugins.manager import PluginManager
+from cleveragents.infrastructure.sandbox.checkpoint import CheckpointManager
 from cleveragents.providers.registry import ProviderRegistry, get_provider_registry
 from cleveragents.shared.redaction import redact_value
 from cleveragents.tui.persona.registry import PersonaRegistry
@@ -693,6 +694,11 @@ class Container(containers.DeclarativeContainer):
         lock_service=lock_service,
     )
 
+    # Checkpoint Manager - in-memory sandbox snapshots (infra layer)
+    checkpoint_manager = providers.Singleton(
+        CheckpointManager,
+    )
+
     # Checkpoint Service - database-backed via CheckpointRepository
     checkpoint_service = providers.Factory(
         _build_checkpoint_service,

src/cleveragents/application/services/plan_executor.py

@@ -609,12 +609,41 @@ class PlanExecutor:
             meta = dict(metadata or {})
             if sandbox.context is not None:
                 meta["sandbox_path"] = sandbox.context.sandbox_path
-            return self._checkpoint_manager.create_checkpoint(
+            checkpoint = self._checkpoint_manager.create_checkpoint(
                 sandbox=sandbox,
                 plan_id=plan_id,
                 phase=phase,
                 metadata=meta,
             )
+
+            # Bridge infra→domain: persist checkpoint ID on the plan
+            # so plan status and plan rollback can reference it.
+            if checkpoint is not None:
+                try:
+                    plan = self._lifecycle.get_plan(plan_id)
+                    plan.last_checkpoint_id = checkpoint.checkpoint_id
+                    self._lifecycle._commit_plan(plan)
+                    self._logger.info(
+                        "Checkpoint created and persisted",
+                        plan_id=plan_id,
+                        checkpoint_id=checkpoint.checkpoint_id,
+                        phase=phase,
+                    )
+                except Exception as exc:
+                    self._logger.warning(
+                        "Checkpoint created but plan update failed",
+                        plan_id=plan_id,
+                        phase=phase,
+                        checkpoint_id=getattr(checkpoint, "checkpoint_id", None),
+                        exc_info=True,
+                    )
+                    raise PlanError(
+                        f"Failed to persist checkpoint metadata for plan {plan_id}"
+                    ) from exc
+
+            return checkpoint
+        except PlanError:
+            raise
         except Exception:
             self._logger.debug(
                 "Checkpoint creation failed (non-fatal)",

src/cleveragents/cli/commands/plan.py

@@ -1740,11 +1740,14 @@ def _get_plan_executor(
         resource_registry=container.resource_registry_service(),
     )
 
+    checkpoint_manager = container.checkpoint_manager()
+
     return PlanExecutor(
         lifecycle_service=lifecycle_service,
         strategize_actor=strategize_actor,
         execute_actor=execute_actor,
         sandbox_root=sandbox_root,
+        checkpoint_manager=checkpoint_manager,
     )
 
 
@@ -2476,6 +2479,12 @@ def execute_plan(
                     plan_id,
                 )
 
+        # Notify A2A facade for protocol bookkeeping.
+        # Use plan.status (read-only) instead of plan.execute (transition)
+        # to avoid a duplicate execute→execute transition error when the
+        # plan has already completed the execute phase.
+        _notify_facade("plan.status", {"plan_id": plan_id})
+
         if fmt != OutputFormat.RICH.value:
             execute_elapsed_ms = int(
                 (datetime.now() - execute_wall_start).total_seconds() * 1000

src/cleveragents/domain/models/core/resource.py

@@ -35,7 +35,7 @@ import re
 from datetime import UTC, datetime
 from enum import StrEnum
 
-from pydantic import BaseModel, ConfigDict, Field, field_validator
+from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator
 
 # ULID: 26 characters, Crockford's base32
 ULID_PATTERN = r"^[0-9A-HJKMNP-TV-Z]{26}$"
@@ -98,12 +98,29 @@ class ResourceCapabilities(BaseModel):
         description="Whether the resource supports sandbox isolation",
     )
     checkpointable: bool = Field(
-        default=False,
+        default=True,
         description="Whether the resource supports checkpoint/rollback",
     )
 
     model_config = ConfigDict(frozen=True)
 
+    @model_validator(mode="before")
+    @classmethod
+    def _default_checkpointable(cls, data: object) -> object:
+        if isinstance(data, dict) and "checkpointable" not in data:
+            writable = data.get("writable", True)
+            sandboxable = data.get("sandboxable", True)
+            data["checkpointable"] = bool(writable and sandboxable)
+        return data
+
+    @model_validator(mode="after")
+    def _validate_checkpointable(self) -> ResourceCapabilities:
+        if self.checkpointable and not self.writable:
+            raise ValueError("Non-writable resources cannot be checkpointable")
+        if self.checkpointable and not self.sandboxable:
+            raise ValueError("Non-sandboxable resources cannot be checkpointable")
+        return self
+
 
 class Resource(BaseModel):
     """Domain model for a Resource Registry entry (ResourceRecord).

src/cleveragents/domain/models/core/tool.py

@@ -251,6 +251,15 @@ class ToolCapability(BaseModel):
         default=None, description="Named cost profile for budgeting"
     )
 
+    @model_validator(mode="before")
+    @classmethod
+    def _default_checkpointable(cls, data: object) -> object:
+        if isinstance(data, dict) and "checkpointable" not in data:
+            writes = data.get("writes", False)
+            read_only = data.get("read_only", False)
+            data["checkpointable"] = bool(writes and not read_only)
+        return data
+
     @model_validator(mode="after")
     def _enforce_read_only(self) -> ToolCapability:
         """If read_only is True, writes and checkpointable must be False."""