Proposal: update specification — ResourceHandler protocol signatures and result types #4616

New issue

Open

opened 2026-04-08 17:37:22 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-08 17:37:22 +00:00

Owner

Spec Update Proposal

Triggered by: Merged PR #1067 (feat(resource): implement ResourceHandler sandbox and checkpoint methods)

What Changed in the Implementation

PR #1067 implemented the ResourceHandler sandbox and checkpoint methods. The implementation uses a significantly different (and better) API design than what the spec describes:

Spec (docs/specification.md, lines 25228–25254):

class ResourceHandler(Protocol):
    def create_sandbox(self, resource: ResourceRecord) -> Sandbox: ...
    def create_checkpoint(self, sandbox: Sandbox) -> Checkpoint: ...
    def rollback_to(self, sandbox: Sandbox, checkpoint: Checkpoint) -> None: ...
    def project_access(
        self,
        binding_resource: ResourceRecord,
        target_resource: ResourceRecord,
        containment_path: list[ResourceRecord],
        sandbox: Sandbox | None,
    ) -> AccessProjection | None: ...

Implementation (src/cleveragents/resource/handlers/protocol.py):

class ResourceHandler(Protocol):
    def create_sandbox(
        self, *, resource: Resource, plan_id: str, sandbox_manager: SandboxManager,
    ) -> SandboxResult: ...
    def create_checkpoint(
        self, *, resource: Resource, plan_id: str, sandbox_manager: SandboxManager, phase: str = "execution",
    ) -> CheckpointResult: ...
    def rollback_to(
        self, *, resource: Resource, plan_id: str, checkpoint_id: str, sandbox_manager: SandboxManager,
    ) -> RollbackResult: ...
    def project_access(
        self, *, resource: Resource, principal: str, action: str = "read", project_id: str = "",
    ) -> AccessResult: ...

The implementation also introduces concrete frozen dataclass result types:

SandboxResult — sandbox_id, sandbox_path, strategy, created, message
CheckpointResult — checkpoint_id, plan_id, timestamp, snapshot_path, message
RollbackResult — success, checkpoint_id, restored_files, message
AccessResult — permitted, principal, action, scope, reason

Why the Implementation is Better

Keyword-only arguments (*) prevent positional argument confusion across complex method signatures
Explicit plan_id and sandbox_manager parameters make dependencies explicit rather than relying on a Sandbox object that implicitly carries these
checkpoint_id: str instead of Checkpoint object for rollback_to — simpler, avoids requiring callers to hold the full checkpoint object
Concrete result types (SandboxResult, CheckpointResult, RollbackResult, AccessResult) instead of abstract Sandbox, Checkpoint, AccessProjection — these are frozen dataclasses with clear fields, making the API self-documenting
project_access simplified — the spec's binding_resource/target_resource/containment_path model was overly complex; the implementation uses a simpler principal/action/project_id model that covers the actual use cases

Spec Sections Affected

Section: Resource Handler Interface (lines 25188–25255) — the ResourceHandler protocol code block needs to be updated to match the implementation signatures
Section: Built-in Resource Handlers (lines 25257–25284) — the table may need minor updates to reflect FsDirectoryHandler (not FilesystemHandler) for fs-directory

Proposed Changes

Current spec text (lines 25228–25254):

    def create_sandbox(self, resource: ResourceRecord) -> Sandbox:
        """Create a sandbox for this resource using its type's strategy."""
        ...

    def create_checkpoint(self, sandbox: Sandbox) -> Checkpoint:
        """Create a checkpoint within the sandbox."""
        ...

    def rollback_to(self, sandbox: Sandbox, checkpoint: Checkpoint) -> None:
        """Roll back sandbox state to a checkpoint."""
        ...

    def project_access(
        self,
        binding_resource: ResourceRecord,
        target_resource: ResourceRecord,
        containment_path: list[ResourceRecord],
        sandbox: Sandbox | None,
    ) -> AccessProjection | None:
        """Compute how to reach target_resource from binding_resource.
        ...
        """
        ...

Proposed replacement:

    def create_sandbox(
        self,
        *,
        resource: Resource,
        plan_id: str,
        sandbox_manager: SandboxManager,
    ) -> SandboxResult:
        """Create an isolated sandbox for the resource (idempotent).
        
        Returns a SandboxResult with sandbox_id, sandbox_path, strategy,
        created (idempotency flag), and message.
        """
        ...

    def create_checkpoint(
        self,
        *,
        resource: Resource,
        plan_id: str,
        sandbox_manager: SandboxManager,
        phase: str = "execution",
    ) -> CheckpointResult:
        """Create a checkpoint of the current resource state.
        
        Returns a CheckpointResult with checkpoint_id, plan_id, timestamp,
        snapshot_path, and message.
        """
        ...

    def rollback_to(
        self,
        *,
        resource: Resource,
        plan_id: str,
        checkpoint_id: str,
        sandbox_manager: SandboxManager,
    ) -> RollbackResult:
        """Rollback a resource to a prior checkpoint state.
        
        Returns a RollbackResult with success, checkpoint_id, restored_files,
        and message.
        """
        ...

    def project_access(
        self,
        *,
        resource: Resource,
        principal: str,
        action: str = "read",
        project_id: str = "",
    ) -> AccessResult:
        """Check access permissions for the resource.
        
        Returns an AccessResult with permitted, principal, action, scope,
        and reason.
        """
        ...

Also add a new subsection documenting the result types (SandboxResult, CheckpointResult, RollbackResult, AccessResult) before the protocol definition.

Rationale

The implementation discovered that the spec's abstract Sandbox/Checkpoint/AccessProjection types were unnecessary indirection. The concrete result dataclasses are simpler, more testable, and make the API self-documenting. The keyword-only argument style is consistent with the rest of the codebase's Python conventions.

This is a major spec change (alters the core ResourceHandler protocol interface) and requires human approval before being committed.

Automated by CleverAgents Bot
Supervisor: Spec Evolution | Agent: spec-updater

## Spec Update Proposal **Triggered by:** Merged PR #1067 (`feat(resource): implement ResourceHandler sandbox and checkpoint methods`) ### What Changed in the Implementation PR #1067 implemented the `ResourceHandler` sandbox and checkpoint methods. The implementation uses a significantly different (and better) API design than what the spec describes: **Spec (docs/specification.md, lines 25228–25254):** ```python class ResourceHandler(Protocol): def create_sandbox(self, resource: ResourceRecord) -> Sandbox: ... def create_checkpoint(self, sandbox: Sandbox) -> Checkpoint: ... def rollback_to(self, sandbox: Sandbox, checkpoint: Checkpoint) -> None: ... def project_access( self, binding_resource: ResourceRecord, target_resource: ResourceRecord, containment_path: list[ResourceRecord], sandbox: Sandbox | None, ) -> AccessProjection | None: ... ``` **Implementation (src/cleveragents/resource/handlers/protocol.py):** ```python class ResourceHandler(Protocol): def create_sandbox( self, *, resource: Resource, plan_id: str, sandbox_manager: SandboxManager, ) -> SandboxResult: ... def create_checkpoint( self, *, resource: Resource, plan_id: str, sandbox_manager: SandboxManager, phase: str = "execution", ) -> CheckpointResult: ... def rollback_to( self, *, resource: Resource, plan_id: str, checkpoint_id: str, sandbox_manager: SandboxManager, ) -> RollbackResult: ... def project_access( self, *, resource: Resource, principal: str, action: str = "read", project_id: str = "", ) -> AccessResult: ... ``` The implementation also introduces concrete frozen dataclass result types: - `SandboxResult` — `sandbox_id`, `sandbox_path`, `strategy`, `created`, `message` - `CheckpointResult` — `checkpoint_id`, `plan_id`, `timestamp`, `snapshot_path`, `message` - `RollbackResult` — `success`, `checkpoint_id`, `restored_files`, `message` - `AccessResult` — `permitted`, `principal`, `action`, `scope`, `reason` ### Why the Implementation is Better 1. **Keyword-only arguments** (`*`) prevent positional argument confusion across complex method signatures 2. **Explicit `plan_id` and `sandbox_manager` parameters** make dependencies explicit rather than relying on a `Sandbox` object that implicitly carries these 3. **`checkpoint_id: str` instead of `Checkpoint` object** for `rollback_to` — simpler, avoids requiring callers to hold the full checkpoint object 4. **Concrete result types** (`SandboxResult`, `CheckpointResult`, `RollbackResult`, `AccessResult`) instead of abstract `Sandbox`, `Checkpoint`, `AccessProjection` — these are frozen dataclasses with clear fields, making the API self-documenting 5. **`project_access` simplified** — the spec's `binding_resource/target_resource/containment_path` model was overly complex; the implementation uses a simpler `principal/action/project_id` model that covers the actual use cases ### Spec Sections Affected - **Section: Resource Handler Interface** (lines 25188–25255) — the `ResourceHandler` protocol code block needs to be updated to match the implementation signatures - **Section: Built-in Resource Handlers** (lines 25257–25284) — the table may need minor updates to reflect `FsDirectoryHandler` (not `FilesystemHandler`) for `fs-directory` ### Proposed Changes **Current spec text (lines 25228–25254):** ```python def create_sandbox(self, resource: ResourceRecord) -> Sandbox: """Create a sandbox for this resource using its type's strategy.""" ... def create_checkpoint(self, sandbox: Sandbox) -> Checkpoint: """Create a checkpoint within the sandbox.""" ... def rollback_to(self, sandbox: Sandbox, checkpoint: Checkpoint) -> None: """Roll back sandbox state to a checkpoint.""" ... def project_access( self, binding_resource: ResourceRecord, target_resource: ResourceRecord, containment_path: list[ResourceRecord], sandbox: Sandbox | None, ) -> AccessProjection | None: """Compute how to reach target_resource from binding_resource. ... """ ... ``` **Proposed replacement:** ```python def create_sandbox( self, *, resource: Resource, plan_id: str, sandbox_manager: SandboxManager, ) -> SandboxResult: """Create an isolated sandbox for the resource (idempotent). Returns a SandboxResult with sandbox_id, sandbox_path, strategy, created (idempotency flag), and message. """ ... def create_checkpoint( self, *, resource: Resource, plan_id: str, sandbox_manager: SandboxManager, phase: str = "execution", ) -> CheckpointResult: """Create a checkpoint of the current resource state. Returns a CheckpointResult with checkpoint_id, plan_id, timestamp, snapshot_path, and message. """ ... def rollback_to( self, *, resource: Resource, plan_id: str, checkpoint_id: str, sandbox_manager: SandboxManager, ) -> RollbackResult: """Rollback a resource to a prior checkpoint state. Returns a RollbackResult with success, checkpoint_id, restored_files, and message. """ ... def project_access( self, *, resource: Resource, principal: str, action: str = "read", project_id: str = "", ) -> AccessResult: """Check access permissions for the resource. Returns an AccessResult with permitted, principal, action, scope, and reason. """ ... ``` Also add a new subsection documenting the result types (`SandboxResult`, `CheckpointResult`, `RollbackResult`, `AccessResult`) before the protocol definition. ### Rationale The implementation discovered that the spec's abstract `Sandbox`/`Checkpoint`/`AccessProjection` types were unnecessary indirection. The concrete result dataclasses are simpler, more testable, and make the API self-documenting. The keyword-only argument style is consistent with the rest of the codebase's Python conventions. This is a **major spec change** (alters the core ResourceHandler protocol interface) and requires human approval before being committed. --- **Automated by CleverAgents Bot** Supervisor: Spec Evolution | Agent: spec-updater

HAL9000 referenced this issue

2026-04-08 18:43:13 +00:00

[Automated] CleverAgents Build Session - 2026-04-08 #4743

HAL9000 referenced this issue

2026-04-08 19:33:22 +00:00

[Automated] CleverAgents Build Session - 2026-04-08 #4799

freemo added the

Rows
Columns