From bdd1ea4f3af55b1df514af553cf98d8f74f45b19 Mon Sep 17 00:00:00 2001 From: Hamza Khyari Date: Fri, 17 Apr 2026 11:46:38 +0000 Subject: [PATCH] fix(checkpoint): wire CheckpointManager into PlanExecutor execution path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- CHANGELOG.md | 14 + features/checkpoint_wiring.feature | 63 ++++ features/consolidated_resource.feature | 9 +- features/consolidated_skill.feature | 2 +- features/steps/checkpoint_wiring_steps.py | 297 ++++++++++++++++++ .../steps/resource_registry_model_steps.py | 10 + robot/helper_skill_flatten.py | 4 +- src/cleveragents/application/container.py | 6 + .../application/services/plan_executor.py | 31 +- src/cleveragents/cli/commands/plan.py | 9 + .../domain/models/core/resource.py | 21 +- src/cleveragents/domain/models/core/tool.py | 9 + 12 files changed, 465 insertions(+), 10 deletions(-) create mode 100644 features/checkpoint_wiring.feature create mode 100644 features/steps/checkpoint_wiring_steps.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 567d698ed..33a28137f 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/features/checkpoint_wiring.feature b/features/checkpoint_wiring.feature new file mode 100644 index 000000000..b7df9dbe9 --- /dev/null +++ b/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 diff --git a/features/consolidated_resource.feature b/features/consolidated_resource.feature index 977d2a2a9..c70e04727 100644 --- a/features/consolidated_resource.feature +++ b/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 diff --git a/features/consolidated_skill.feature b/features/consolidated_skill.feature index 1bb8e3b40..450cc980c 100644 --- a/features/consolidated_skill.feature +++ b/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 diff --git a/features/steps/checkpoint_wiring_steps.py b/features/steps/checkpoint_wiring_steps.py new file mode 100644 index 000000000..dcbd1cfed --- /dev/null +++ b/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 diff --git a/features/steps/resource_registry_model_steps.py b/features/steps/resource_registry_model_steps.py index 62402819c..04862f1f2 100644 --- a/features/steps/resource_registry_model_steps.py +++ b/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.""" diff --git a/robot/helper_skill_flatten.py b/robot/helper_skill_flatten.py index 67e053d0d..363e78266 100644 --- a/robot/helper_skill_flatten.py +++ b/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): diff --git a/src/cleveragents/application/container.py b/src/cleveragents/application/container.py index e86956146..2a2309ab0 100644 --- a/src/cleveragents/application/container.py +++ b/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, diff --git a/src/cleveragents/application/services/plan_executor.py b/src/cleveragents/application/services/plan_executor.py index a2dc39af1..dd6f20d7d 100644 --- a/src/cleveragents/application/services/plan_executor.py +++ b/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)", diff --git a/src/cleveragents/cli/commands/plan.py b/src/cleveragents/cli/commands/plan.py index 116213619..9f46bdf35 100644 --- a/src/cleveragents/cli/commands/plan.py +++ b/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 diff --git a/src/cleveragents/domain/models/core/resource.py b/src/cleveragents/domain/models/core/resource.py index c0cf4b773..d2100490f 100644 --- a/src/cleveragents/domain/models/core/resource.py +++ b/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). diff --git a/src/cleveragents/domain/models/core/tool.py b/src/cleveragents/domain/models/core/tool.py index 4d2e3aaba..0ff79b3a6 100644 --- a/src/cleveragents/domain/models/core/tool.py +++ b/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.""" -- 2.52.0