fix(checkpoint): wire CheckpointManager into PlanExecutor execution path
CI / push-validation (pull_request) Successful in 10s
CI / build (pull_request) Successful in 16s
CI / helm (pull_request) Successful in 16s
CI / typecheck (pull_request) Successful in 30s
CI / lint (pull_request) Successful in 35s
CI / quality (pull_request) Successful in 35s
CI / security (pull_request) Successful in 50s
CI / integration_tests (pull_request) Successful in 4m3s
CI / e2e_tests (pull_request) Successful in 4m14s
CI / unit_tests (pull_request) Successful in 5m13s
CI / docker (pull_request) Successful in 8s
CI / coverage (pull_request) Successful in 11m2s
CI / status-check (pull_request) Successful in 1s

CheckpointManager was never wired into PlanExecutor — the CLI factory
constructed PlanExecutor without a checkpoint_manager (defaulted to
None), silently skipping all checkpoint hooks.

Fix:
- Register CheckpointManager as Singleton in DI container
- Resolve container singleton in _get_plan_executor() and pass to
  PlanExecutor constructor
- Bridge infra→domain: _try_create_checkpoint() now persists
  last_checkpoint_id on the plan via _commit_plan(), raises PlanError
  if persistence fails
- Default checkpointable=True for writable+sandboxable resources and
  write-capable tools (model_validators on ResourceCapabilities and
  ToolCapability)
- Validate that non-writable/non-sandboxable resources cannot be
  checkpointable (ValueError guard)
- Add post-execute A2A facade notification using plan.status to avoid
  duplicate execute→execute transition errors

Tests:
- 10 Behave scenarios covering DI wiring, singleton identity, checkpoint
  creation, plan metadata update, rollback, graceful fallback, no-arg
  constructor, capability defaults (positive + 2 negative)
- Updated consolidated_resource, consolidated_skill, and Robot
  helper_skill_flatten for new checkpointable defaults

ISSUES CLOSED: #1253
This commit is contained in:
2026-04-17 11:46:38 +00:00
parent e2b127b7e5
commit bdd1ea4f3a
12 changed files with 465 additions and 10 deletions
+14
View File
@@ -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
+63
View File
@@ -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
+4 -5
View File
@@ -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
+1 -1
View File
@@ -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
+297
View File
@@ -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
@@ -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."""
+3 -1
View File
@@ -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):
@@ -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,
@@ -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)",
+9
View File
@@ -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
@@ -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).
@@ -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."""