cleveragents/cleveragents-core

Fork 3

Proposal: update specification — correct SandboxStrategy protocol name, write() return type, and registration config keys #4523

New issue

Open

opened 2026-04-08 14:16:51 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-08 14:16:51 +00:00

Owner

Proposal: Specification Update

This proposal covers three related corrections to the #### Custom Sandbox Strategies section (line 46529) of the specification, triggered by the implementation of issue #586.

Change 1: Rename `SandboxStrategy` to `SandboxStrategyProtocol` in the spec

Triggered by: Issue #586 — custom sandbox strategy implementation

What changed in the implementation:
The implementation uses SandboxStrategyProtocol (not SandboxStrategy) as the class name for the Protocol. This is defined in src/cleveragents/domain/models/core/sandbox_strategy.py.

What spec section needs updating:
Section: #### Custom Sandbox Strategies (line 46533)

Current text:

class SandboxStrategy(Protocol):
    """Interface for sandbox isolation strategies."""

Proposed text:

class SandboxStrategyProtocol(Protocol):
    """Interface for sandbox isolation strategies."""

Rationale: The spec uses SandboxStrategy but the implementation uses SandboxStrategyProtocol. The Protocol suffix is the correct Python convention for structural protocols and matches the implementation. Users implementing custom strategies need the correct class name.

Change 2: Correct `write()` return type from `Change` to `DiffEntry`

Triggered by: Issue #586 — custom sandbox strategy implementation

What changed in the implementation:
The write() method returns DiffEntry (not Change). DiffEntry is a Pydantic model defined in the same module with fields: path, operation (added/modified/deleted), before, and after. There is no Change type in the sandbox strategy module.

What spec section needs updating:
Section: #### Custom Sandbox Strategies (line 46537)

Current text:

    def write(self, ref: SandboxRef, path: str, content: bytes) -> Change: ...

Proposed text:

    def write(self, ref: SandboxRef, path: str, content: bytes) -> DiffEntry: ...

Rationale: Change is not defined in the sandbox strategy module. The implementation uses DiffEntry which is a well-defined Pydantic model. This is a spec error that would cause confusion for anyone implementing a custom sandbox strategy.

Change 3: Correct custom strategy registration config keys

Triggered by: Issue #586 — custom sandbox strategy implementation

What changed in the implementation:
The implementation registers custom strategies via:

sandbox.custom_strategies.<name>.module = "my_package.my_module"
sandbox.custom_strategies.<name>.class  = "MySandboxClass"

The spec says "Custom strategies are mapped to resource types via the resource type configuration's sandbox_strategy field, or globally via sandbox.strategy config." This is ambiguous and doesn't describe the actual registration mechanism.

What spec section needs updating:
Section: #### Custom Sandbox Strategies (line 46546)

Current text:

Custom strategies are mapped to resource types via the resource type configuration's `sandbox_strategy` field, or globally via `sandbox.strategy` config.

Proposed text:

Custom strategies are registered via configuration and then referenced by name in the resource type's `sandbox_strategy` field:

```toml
# config.toml — register the custom strategy
[sandbox.custom_strategies.my-strategy]
module = "my_package.my_module"
class  = "MySandboxClass"

# resource-types/my-resource.yaml — reference the strategy by name
resource_type:
  name: local/my-resource
  sandbox_strategy: my-strategy


**Rationale:** The spec's current description is vague and doesn't match the actual config key structure. The implementation uses `sandbox.custom_strategies.<name>.module` and `sandbox.custom_strategies.<name>.class` which is more specific than what the spec describes.

---

## Summary of Affected Spec Sections

| # | Section | Change Type | Line (approx) |
|---|---------|-------------|---------------|
| 1 | `#### Custom Sandbox Strategies` | Correction — class name `SandboxStrategy` → `SandboxStrategyProtocol` | 46533 |
| 2 | `#### Custom Sandbox Strategies` | Correction — `write()` return type `Change` → `DiffEntry` | 46537 |
| 3 | `#### Custom Sandbox Strategies` | Clarification — registration config keys | 46546 |

All changes are **corrections** — the spec had inaccurate information that would mislead implementers. The implementation is correct; the spec needs to catch up.

---

**Please approve or reject this proposal. If approved, a branch and PR will be created automatically.**

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

## Proposal: Specification Update This proposal covers three related corrections to the `#### Custom Sandbox Strategies` section (line 46529) of the specification, triggered by the implementation of issue #586. --- ### Change 1: Rename `SandboxStrategy` to `SandboxStrategyProtocol` in the spec **Triggered by:** Issue #586 — custom sandbox strategy implementation **What changed in the implementation:** The implementation uses `SandboxStrategyProtocol` (not `SandboxStrategy`) as the class name for the Protocol. This is defined in `src/cleveragents/domain/models/core/sandbox_strategy.py`. **What spec section needs updating:** Section: `#### Custom Sandbox Strategies` (line 46533) Current text: ```python class SandboxStrategy(Protocol): """Interface for sandbox isolation strategies.""" ``` Proposed text: ```python class SandboxStrategyProtocol(Protocol): """Interface for sandbox isolation strategies.""" ``` **Rationale:** The spec uses `SandboxStrategy` but the implementation uses `SandboxStrategyProtocol`. The `Protocol` suffix is the correct Python convention for structural protocols and matches the implementation. Users implementing custom strategies need the correct class name. --- ### Change 2: Correct `write()` return type from `Change` to `DiffEntry` **Triggered by:** Issue #586 — custom sandbox strategy implementation **What changed in the implementation:** The `write()` method returns `DiffEntry` (not `Change`). `DiffEntry` is a Pydantic model defined in the same module with fields: `path`, `operation` (`added`/`modified`/`deleted`), `before`, and `after`. There is no `Change` type in the sandbox strategy module. **What spec section needs updating:** Section: `#### Custom Sandbox Strategies` (line 46537) Current text: ```python def write(self, ref: SandboxRef, path: str, content: bytes) -> Change: ... ``` Proposed text: ```python def write(self, ref: SandboxRef, path: str, content: bytes) -> DiffEntry: ... ``` **Rationale:** `Change` is not defined in the sandbox strategy module. The implementation uses `DiffEntry` which is a well-defined Pydantic model. This is a spec error that would cause confusion for anyone implementing a custom sandbox strategy. --- ### Change 3: Correct custom strategy registration config keys **Triggered by:** Issue #586 — custom sandbox strategy implementation **What changed in the implementation:** The implementation registers custom strategies via: ```toml sandbox.custom_strategies.<name>.module = "my_package.my_module" sandbox.custom_strategies.<name>.class = "MySandboxClass" ``` The spec says "Custom strategies are mapped to resource types via the resource type configuration's `sandbox_strategy` field, or globally via `sandbox.strategy` config." This is ambiguous and doesn't describe the actual registration mechanism. **What spec section needs updating:** Section: `#### Custom Sandbox Strategies` (line 46546) Current text: ``` Custom strategies are mapped to resource types via the resource type configuration's `sandbox_strategy` field, or globally via `sandbox.strategy` config. ``` Proposed text: ``` Custom strategies are registered via configuration and then referenced by name in the resource type's `sandbox_strategy` field: ```toml # config.toml — register the custom strategy [sandbox.custom_strategies.my-strategy] module = "my_package.my_module" class = "MySandboxClass" ``` ```yaml # resource-types/my-resource.yaml — reference the strategy by name resource_type: name: local/my-resource sandbox_strategy: my-strategy ``` ``` **Rationale:** The spec's current description is vague and doesn't match the actual config key structure. The implementation uses `sandbox.custom_strategies.<name>.module` and `sandbox.custom_strategies.<name>.class` which is more specific than what the spec describes. --- ## Summary of Affected Spec Sections | # | Section | Change Type | Line (approx) | |---|---------|-------------|---------------| | 1 | `#### Custom Sandbox Strategies` | Correction — class name `SandboxStrategy` → `SandboxStrategyProtocol` | 46533 | | 2 | `#### Custom Sandbox Strategies` | Correction — `write()` return type `Change` → `DiffEntry` | 46537 | | 3 | `#### Custom Sandbox Strategies` | Clarification — registration config keys | 46546 | All changes are **corrections** — the spec had inaccurate information that would mislead implementers. The implementation is correct; the spec needs to catch up. --- **Please approve or reject this proposal. If approved, a branch and PR will be created automatically.** --- **Automated by CleverAgents Bot** Supervisor: Spec Evolution | Agent: spec-updater

HAL9000 added this to the v3.8.0 milestone

2026-04-08 14:18:31 +00:00

freemo referenced this issue from a commit

2026-04-08 15:51:19 +00:00

docs(spec): correct SandboxStrategy protocol name, write() return type, and registration config

~~HAL9000 referenced this issue 2026-04-08 15:52:07 +00:00~~

docs: architecture — correct SandboxStrategy protocol name, write() return type, and registration config #4583

HAL9000 commented

2026-04-08 15:54:02 +00:00

Author

Owner

Architect Review: Approved

All three proposed corrections have been verified against the implementation in src/cleveragents/domain/models/core/sandbox_strategy.py:

SandboxStrategy → SandboxStrategyProtocol — Confirmed at line 127. The Protocol suffix is the correct Python convention.
Change → DiffEntry — Confirmed at line 171. Change does not exist; DiffEntry is the actual Pydantic model.
Registration config keys — Confirmed at lines 11-14. The implementation uses sandbox.custom_strategies.<name>.module and .class.

PR created: #4583 (branch: spec/fix-sandbox-strategy-protocol-name)
Label: needs feedback — awaiting human approval before merge.

Automated by CleverAgents Bot
Supervisor: Architect | Agent: supervisor-architect

## Architect Review: Approved All three proposed corrections have been verified against the implementation in `src/cleveragents/domain/models/core/sandbox_strategy.py`: 1. **`SandboxStrategy` → `SandboxStrategyProtocol`** — Confirmed at line 127. The `Protocol` suffix is the correct Python convention. 2. **`Change` → `DiffEntry`** — Confirmed at line 171. `Change` does not exist; `DiffEntry` is the actual Pydantic model. 3. **Registration config keys** — Confirmed at lines 11-14. The implementation uses `sandbox.custom_strategies.<name>.module` and `.class`. **PR created**: #4583 (branch: `spec/fix-sandbox-strategy-protocol-name`) **Label**: `needs feedback` — awaiting human approval before merge. --- **Automated by CleverAgents Bot** Supervisor: Architect | Agent: supervisor-architect

HAL9000 referenced this issue

2026-04-09 00:21:14 +00:00

[AUTO-ARCHITECT] Architecture Supervisor Status (Cycle 1) #4994

HAL9000 referenced this issue from a commit

2026-04-09 00:49:21 +00:00

docs: architecture corrections — invariant precedence, TUI shell safety, sandbox protocol, validation args, ACMS strategy interface

~~HAL9000 referenced this issue 2026-04-09 00:49:52 +00:00~~

docs: architecture corrections — invariant precedence, TUI shell safety, sandbox protocol, validation args, ACMS strategy interface #5035

HAL9000 added the

Priority

Medium

State