UAT: SandboxFactory.create_sandbox never uses the custom strategy registry — custom sandbox strategies registered via SandboxStrategyRegistry can never be instantiated #2951

Open
opened 2026-04-05 02:55:12 +00:00 by freemo · 1 comment
Owner

Background

The SandboxFactory class accepts a custom_registry: SandboxStrategyRegistry | None parameter in its constructor, and the spec (§ Custom Sandbox Strategies, lines 46171–46186) requires that custom strategies registered via configuration can be resolved and instantiated at runtime. However, the create_sandbox method never consults self._custom_registry — it only checks built-in strategy names and raises ValueError for anything unknown.

This means the entire custom strategy extension point is silently broken: users who register custom sandbox strategies via SandboxStrategyRegistry and pass the registry to SandboxFactory will always receive a ValueError at runtime, with no indication that the registry was ignored.

Current Behavior

In src/cleveragents/infrastructure/sandbox/factory.py lines 102–162, create_sandbox checks for none, git_worktree, copy_on_write, transaction_rollback, overlay, snapshot — and raises ValueError for anything else. The self._custom_registry field is stored in __init__ (lines 88–100) but is never consulted in create_sandbox.

The helper methods has_custom_strategy() and get_custom_strategy_class() (lines 166–190) exist but are dead code from the perspective of create_sandbox.

Steps to reproduce:

  1. Create a SandboxStrategyRegistry and register a custom strategy class
  2. Create a SandboxFactory(custom_registry=registry)
  3. Call factory.create_sandbox(resource_id="r1", original_path="/tmp", sandbox_strategy="my_custom")
  4. Observe ValueError: Unknown sandbox strategy: my_custom instead of the custom strategy being instantiated

Affected files:

  • src/cleveragents/infrastructure/sandbox/factory.py lines 102–162 (create_sandbox — missing registry fallback)
  • src/cleveragents/infrastructure/sandbox/factory.py lines 88–100 (__init__ — stores registry but it is never used)
  • src/cleveragents/infrastructure/sandbox/factory.py lines 166–190 (has_custom_strategy, get_custom_strategy_class — dead code)

Expected Behavior

Per spec § Custom Sandbox Strategies (lines 46171–46186): when sandbox_strategy is not a built-in strategy name, the factory should fall back to self._custom_registry to look up and instantiate the custom strategy class. If the strategy is also not found in the custom registry, only then should a ValueError be raised.

Acceptance Criteria

  • create_sandbox consults self._custom_registry after failing to match a built-in strategy name
  • A custom strategy registered via SandboxStrategyRegistry and passed to SandboxFactory is correctly resolved and instantiated
  • ValueError is only raised when the strategy name is not found in either built-in strategies or the custom registry
  • has_custom_strategy() and get_custom_strategy_class() are exercised by create_sandbox (no longer dead code)
  • Unit tests cover: custom strategy resolution, fallback to ValueError when not in registry, built-in strategies unaffected
  • All nox stages pass; coverage ≥ 97%

Supporting Information

  • Parent Epic: #362 (Epic: Security & Safety Hardening)
  • Spec reference: § Custom Sandbox Strategies, lines 46171–46186
  • Discovered during UAT testing of the Sandbox & Checkpoint Safety feature area

Metadata

  • Branch: fix/sandbox-factory-custom-registry-lookup
  • Commit Message: fix(sandbox): consult custom_registry in SandboxFactory.create_sandbox
  • Milestone: v3.3.0
  • Parent Epic: #362

Subtasks

  • Audit create_sandbox in factory.py and identify the exact insertion point for registry fallback
  • Implement registry fallback: after built-in strategy lookup fails, call self._custom_registry.get_custom_strategy_class(sandbox_strategy) (if registry is set) and instantiate the result
  • Ensure ValueError message is informative when strategy is not found in either source
  • Tests (unit): Add scenarios covering custom strategy resolution via registry, missing strategy with registry set, missing strategy without registry
  • Tests (unit): Verify has_custom_strategy and get_custom_strategy_class are no longer dead code
  • Verify coverage ≥ 97% via nox -s coverage_report
  • Run nox (all default sessions), fix any errors

Definition of Done

This issue is complete when:

  • All subtasks above are completed and checked off.
  • A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation.
  • The commit is pushed to the remote on the branch matching the Branch in Metadata exactly.
  • The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.
  • All nox stages pass.
  • Coverage >= 97%

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-new-issue-creator

## Background The `SandboxFactory` class accepts a `custom_registry: SandboxStrategyRegistry | None` parameter in its constructor, and the spec (§ Custom Sandbox Strategies, lines 46171–46186) requires that custom strategies registered via configuration can be resolved and instantiated at runtime. However, the `create_sandbox` method never consults `self._custom_registry` — it only checks built-in strategy names and raises `ValueError` for anything unknown. This means the entire custom strategy extension point is silently broken: users who register custom sandbox strategies via `SandboxStrategyRegistry` and pass the registry to `SandboxFactory` will always receive a `ValueError` at runtime, with no indication that the registry was ignored. ## Current Behavior In `src/cleveragents/infrastructure/sandbox/factory.py` lines 102–162, `create_sandbox` checks for `none`, `git_worktree`, `copy_on_write`, `transaction_rollback`, `overlay`, `snapshot` — and raises `ValueError` for anything else. The `self._custom_registry` field is stored in `__init__` (lines 88–100) but is **never consulted** in `create_sandbox`. The helper methods `has_custom_strategy()` and `get_custom_strategy_class()` (lines 166–190) exist but are **dead code** from the perspective of `create_sandbox`. **Steps to reproduce:** 1. Create a `SandboxStrategyRegistry` and register a custom strategy class 2. Create a `SandboxFactory(custom_registry=registry)` 3. Call `factory.create_sandbox(resource_id="r1", original_path="/tmp", sandbox_strategy="my_custom")` 4. Observe `ValueError: Unknown sandbox strategy: my_custom` instead of the custom strategy being instantiated **Affected files:** - `src/cleveragents/infrastructure/sandbox/factory.py` lines 102–162 (`create_sandbox` — missing registry fallback) - `src/cleveragents/infrastructure/sandbox/factory.py` lines 88–100 (`__init__` — stores registry but it is never used) - `src/cleveragents/infrastructure/sandbox/factory.py` lines 166–190 (`has_custom_strategy`, `get_custom_strategy_class` — dead code) ## Expected Behavior Per spec § Custom Sandbox Strategies (lines 46171–46186): when `sandbox_strategy` is not a built-in strategy name, the factory should fall back to `self._custom_registry` to look up and instantiate the custom strategy class. If the strategy is also not found in the custom registry, only then should a `ValueError` be raised. ## Acceptance Criteria - [ ] `create_sandbox` consults `self._custom_registry` after failing to match a built-in strategy name - [ ] A custom strategy registered via `SandboxStrategyRegistry` and passed to `SandboxFactory` is correctly resolved and instantiated - [ ] `ValueError` is only raised when the strategy name is not found in either built-in strategies **or** the custom registry - [ ] `has_custom_strategy()` and `get_custom_strategy_class()` are exercised by `create_sandbox` (no longer dead code) - [ ] Unit tests cover: custom strategy resolution, fallback to `ValueError` when not in registry, built-in strategies unaffected - [ ] All nox stages pass; coverage ≥ 97% ## Supporting Information - Parent Epic: #362 (Epic: Security & Safety Hardening) - Spec reference: § Custom Sandbox Strategies, lines 46171–46186 - Discovered during UAT testing of the Sandbox & Checkpoint Safety feature area ## Metadata - **Branch**: `fix/sandbox-factory-custom-registry-lookup` - **Commit Message**: `fix(sandbox): consult custom_registry in SandboxFactory.create_sandbox` - **Milestone**: v3.3.0 - **Parent Epic**: #362 ## Subtasks - [ ] Audit `create_sandbox` in `factory.py` and identify the exact insertion point for registry fallback - [ ] Implement registry fallback: after built-in strategy lookup fails, call `self._custom_registry.get_custom_strategy_class(sandbox_strategy)` (if registry is set) and instantiate the result - [ ] Ensure `ValueError` message is informative when strategy is not found in either source - [ ] Tests (unit): Add scenarios covering custom strategy resolution via registry, missing strategy with registry set, missing strategy without registry - [ ] Tests (unit): Verify `has_custom_strategy` and `get_custom_strategy_class` are no longer dead code - [ ] Verify coverage ≥ 97% via `nox -s coverage_report` - [ ] Run `nox` (all default sessions), fix any errors ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly. - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. - All nox stages pass. - Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator
freemo added this to the v3.3.0 milestone 2026-04-05 02:55:17 +00:00
Author
Owner

Issue triaged by project owner:

  • State: Verified
  • Priority: Confirmed
  • MoSCoW: Should Have

Valid finding verified during batch triage.


Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **Priority**: Confirmed - **MoSCoW**: Should Have Valid finding verified during batch triage. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Blocks
#362 Epic: Security & Safety Hardening
cleveragents/cleveragents-core
Reference
cleveragents/cleveragents-core#2951
No description provided.