feat: add Azure Container Apps cloud sandbox provider

[thegovind]

Add a cloud sandbox ContainerProvider (Azure Container Apps)

Adds ACASandboxProvider: it runs an OpenEnv server in a hosted sandbox and returns an https:// base URL that EnvClient uses over wss://. It's a new implementation of the existing ContainerProvider contract — providers.py and the client/server protocol are untouched — so it sits alongside DaytonaProvider with zero core changes.

Demo video:

pr-video-opeenv-1.mp4

RFC

RFC 002 gains a vendor-neutral "Cloud Sandbox Providers" section: a capability→OpenEnv mapping, 7 protocol invariants (incl. "a healthy /health is not proof — the exposed URL must proxy the /ws WebSocket upgrade" and base_url lifetime/reconnect), and 6 security invariants for untrusted workloads. Any hosted runtime becomes a provider by subclassing ContainerProvider and satisfying the invariants — no protocol change required.

It works — live, against a real ACA sandbox group

Full reset → step → step → state round-trip over wss:// (the thing /health can't prove):

/health -> 200 ; WS CONNECTED
reset -> reward=0.0 done=False
step1 -> steps=1 reward=1.0
step2 -> steps=2 reward=1.0
state -> {'episode_steps': 2, 'episode_open': True}
ROUND-TRIP PASSED

Default-deny egress (deny_all_egress(allow=[...])) verified to close managed-identity token theft via IMDS:

Target	default-deny	+ PyPI allowlist
IMDS 169.254.169.254	blocked (no route)	still blocked
pypi.org	403	200 (real wheel fetched)
unlisted host	403	403

Reproduce (no ACR needed — bootstraps a bundled server onto a public python-3.11 disk):

export OPENENV_ACA_INTEGRATION=1 OPENENV_ACA_DISK=python-3.11
export AZURE_SUBSCRIPTION_ID=… AZURE_RESOURCE_GROUP=… AZURE_SANDBOX_GROUP=… AZURE_REGION=…
PYTHONPATH=src:envs uv run pytest tests/test_core/test_aca_provider_integration.py -m integration   # 1 passed

Secure by default & minimal

• Explicit anonymous_port=True (public ingress is never implicit), https/wss enforced, exec shell-quoted, sandbox output withheld unless surface_server_logs=True.
• Optional openenv[aca] extra; preview SDK imported lazily behind a private adapter that fake-client unit tests pin to its real shape.
• Tests: 24 provider unit tests + opt-in live integration; tests/test_core → 159 passed / 11 skipped; ruff clean.
• 2 commits (provider, RFC), DCO signed.

Deliberately deferred

Explicit suspend/resume and snapshots stay provider-local until ≥2 providers demonstrate the same semantics (per RFC). Authenticated ingress awaits a client-auth RFC.

cc - @burtenshaw

[Darktex]

Note: This is an automated review by Claude Code, not a human review.

---

Alignment Review Report

Automated Checks

• Lint: N/A — the new files are not in the working tree on branch pr-499; checks run against the diff only
• Debug code: CLEAN — no debug artifacts introduced in this PR
• RFC coverage: PRESENT — RFC 002 amended with "Cloud Sandbox Providers" section; invariants are referenced throughout

---

Tier 1: Fixes Required

T1-1: _start_server shell-exec ambiguity (probable bug)

src/openenv/core/containers/runtime/aca_provider.py, lines 677–688

def _start_server(self, cmd: str) -> None:
    ...
    escaped = shlex.quote(command)
    self._adapter.exec(
        self._sandbox,
        f"nohup bash -c {escaped} > /tmp/openenv-server.log 2>&1 & "
        "echo $! > /tmp/openenv-server.pid",
    )

The full string — including >, 2>&1, &, echo $! — is passed as a single string to sandbox.exec(). Whether the ACA SDK interprets this through a shell or as a raw argument sequence is SDK-dependent. Looking at the real SDK shape validated in test_default_adapter_matches_preview_sdk_shape, sandbox.exec(command, working_directory=...) maps straight through to the sandbox, which — for container-exec APIs generally — typically bypasses the shell. If that is the case here, the backgrounding (&), output redirection, and $! capture will silently not work, and the provider will believe the server started successfully when it did not.

Fix: Wrap the entire command in an explicit sh -c '...' call at the outermost level rather than relying on the SDK to exec via a shell, or explicitly confirm the ACA SDK exec call interprets its first argument through a shell and document this in a comment.

T1-2: Silent misuse of Docker-registry image strings

src/openenv/core/containers/runtime/aca_provider.py, _source_from_image (lines 431–446)

A plain string not prefixed with disk: or disk-id: is silently treated as a public disk name and passed to the ACA SDK as disk=<image>. A caller migrating from a Docker-backed provider who passes "myregistry.azurecr.io/echo-env:latest" will get an opaque ACA SDK error instead of a clear message. Given the docstring says "a plain string … for a public disk image" this is technically documented, but the absence of a sanity check (e.g., warning or rejecting strings that look like OCI references) is a likely foot-gun. A warning for strings containing / or : would help.

T1-3: close() not in ContainerProvider ABC

src/openenv/core/containers/runtime/providers.py (existing), aca_provider.py line 800

ACASandboxProvider.close() is a meaningful resource-cleanup operation (stops the sandbox AND releases the SDK client). ContainerProvider and DaytonaProvider have no close(). Code that holds a ContainerProvider reference cannot call close() without downcasting. This also means orchestration helpers written against ContainerProvider will leak the SDK client. Either:

• Add close() to the ContainerProvider ABC (a non-breaking addition since it can default to calling stop_container()), or
• Document clearly that ACASandboxProvider must not be held as a bare ContainerProvider reference if the caller needs close().

T1-4: integration test skip pattern

tests/test_core/test_aca_provider_integration.py, line 1589

The skip guard if os.environ.get("OPENENV_ACA_INTEGRATION") != "1": pytest.skip(...) is inside the function body rather than expressed as a skipif condition. The test is collected in every run and appears as a skip — which is fine — but consider @pytest.mark.skipif(...) or conftest.py-level mark exclusion for consistency with the repo pattern.

---

Tier 2: Alignment Discussion

ALIGNMENT FLAG: image parameter semantics break the ContainerProvider API contract

• Principle at stake: "Simple Gymnasium-style API" / "Minimize lifecycle deltas" (PRINCIPLES.md); RFC 002 security invariant 4 ("Provider-specific source mapping. Providers document their accepted source forms and never imply a Docker registry image is universally runnable.")
• The concern: ContainerProvider.start_container's image parameter docstring says "Container image name (e.g., 'echo-env:latest')". ACASandboxProvider redefines image to mean an ACA disk name or disk: / disk-id: reference — a fundamentally different concept. RFC 002's new Cloud Sandbox Providers section explicitly allows this divergence, but the base class docstring is not updated, so the ContainerProvider contract still implies a Docker image and nothing prevents a caller from passing a Docker image tag. Update the base class docstring to remove the Docker-specific example and generalize to "provider-specific source identifier".
• Suggested reviewer: @Darktex

ALIGNMENT FLAG: anonymous_port: Optional[bool] lies to the type checker

• Principle at stake: Security invariants S1/S3 from RFC 002 (public exposure must be explicit opt-in)
• The concern: Validating anonymous_port at construction (rather than per-start) is the right call — it's a permanent provider-level posture. But typing it Optional[bool] while rejecting None means None is never valid; the signature lies. A Literal[True] or a custom type would make the security posture statically enforceable. Minor, but security postures should be hard to misconfigure.
• Suggested reviewer: @Darktex

---

Summary

• 3 mechanical issues to fix: shell-exec ambiguity in _start_server (probable functional bug), missing sanity check on Docker-style image strings, and close() not present on the ABC.
• 1 test hygiene note: integration skip pattern.
• 2 alignment points for human review: image parameter contract drift from the base class, and anonymous_port: Optional[bool] lying to the type system.

The RFC amendment is thorough and the security invariant coverage (S1–S6) is exceptionally well-documented. The adapter pattern with a duck-typed fake that asserts real SDK method shapes is exactly the right approach for a preview SDK. The test suite is solid. The shell-exec issue in T1-1 is the highest-priority fix before merging.

---

Automated review by Claude Code | Learn more

[thegovind]

Thanks for the thorough review - addressed them in 73348dd

Tier 1

T1-1 — _start_server shell-exec. Confirmed (not a bug) and documented. The adapter's exec maps to the SDK's executeShellCommand endpoint, whose docstring is "Execute a shell command" and which instructs callers to shlex.quote() interpolated input — i.e. it is shell-interpreted, so the backgrounding (&), redirect, and $! capture work. (This is also what the live reset/step/state round-trip proves: a non-shell exec would have blocked on the foreground server.) I added a comment recording this dependency and a note to re-validate on SDK upgrades. I did not add an sh -c wrapper: it would still rely on the SDK interpreting/splitting the command, so it wouldn't actually remove the shell assumption — documenting the (real, validated) contract is the honest fix.

T1-2 — Docker/OCI image strings. Fixed. _source_from_image now raises a clear, actionable ValueError when a bare string looks like a container reference (contains / or :), instead of passing it to the SDK as a disk name. Explicit disk:<name> / disk-id:<id> sources return before the guard (so slashed disk-id: ARM IDs are unaffected — test_disk_id_source still passes), and disk:<name> is the deliberate escape hatch if you ever have an unusual disk name. New test: test_docker_image_reference_is_rejected.

T1-3 — close() not on the ABC. Partially addressed, intentionally. ACASandboxProvider is now a context manager (__enter__/__exit__) so the sandbox and SDK client are released deterministically (incl. on exceptions — new test_context_manager_closes_on_exception), and the docstring is explicit that close() is provider-local. I deliberately did not add close() to the ContainerProvider ABC, because that changes the core API surface for every provider and is a maintainer call. If you'd like the polymorphic gap closed, here's a backwards-compatible patch I'm happy to fold in (no subclass changes required):

# providers.py — ContainerProvider
def close(self) -> None:
    """Release provider resources. Defaults to stopping the container."""
    self.stop_container()

def __enter__(self) -> "ContainerProvider":
    return self

def __exit__(self, *exc: object) -> None:
    self.close()

Just say the word and I'll add it (and drop the ACA-local context manager in favor of the inherited one).

T1-4 — integration skip. Done — now @pytest.mark.skipif(... OPENENV_ACA_INTEGRATION ...). (The secondary OPENENV_ACA_DISK in-body skip is a config gate that depends on other env, so it stays in-body.)

Tier 2 (alignment)

T2-1 — image contract drift. Done. Generalized the base ContainerProvider.start_container image docstring from "Container image name" to a provider-neutral "container source identifier" (Docker = a registry image; other providers may map it to a provider-specific source). This is docs-only/zero-behavior and, if anything, reinforces the provider-neutral contract. Also made the ACA image docstring explicit that it is a sandbox source, not a Docker image. cc @Darktex if you'd prefer different wording.

T2-2 — anonymous_port: Optional[bool] lies. Done. Typed Literal[True] | None: a static checker now rejects anonymous_port=False, while None stays the explicit "you must opt in" default (runtime validation unchanged, defense-in-depth). cc @Darktex.

All 27 provider unit tests pass; tests/test_core is 162 passed / 11 skipped; ruff clean.

[Darktex]

Note: This is an automated review by Claude Code, not a human review.

---

Alignment Review Report

This PR adds ACASandboxProvider, a new ContainerProvider implementation backed by Azure Container Apps Sandboxes. The provider is well-structured, follows the adapter pattern for testability, and ships with 24 focused unit tests plus an opt-in live integration test. The RFC 002 amendment adds a coherent vendor-neutral capability mapping and security invariant set. No core protocol changes are made.

---

Tier 1: Bugs & Correctness

Automated Checks

• Lint: Could not run directly (new files not on disk), but manual inspection of the diff finds no ruff violations — imports are stdlib-first, then third-party (requests), style is consistent, no bare except clauses.
• Debug code: CLEAN — no print, breakpoint, or unguarded TODOs in the new files.

Issues Found

T1-1: src/openenv/core/containers/runtime/aca_provider.py lines 709–725 — _start_server applies shlex.quote(working_directory) when building command, then applies shlex.quote(command) to the whole compound string. This is correct for the nohup bash -c <escaped> invocation pattern. There is a subtle behavioral gap: a user-supplied cmd containing shell metacharacters is evaluated inside the bash -c subshell. This is acceptable because cmd comes from the trusted orchestrator, not agent input (RFC amendment security invariant S5), but worth a code comment clarifying that cmd is assumed trusted.

T1-2: aca_provider.py line 239 — import requests is at module level, differing from DaytonaProvider's lazy import requests inside wait_for_ready(). requests is a core dependency so there is no import-failure risk, but it is a style inconsistency. Not blocking.

T1-3: stop_container() (lines 816–826) clears _sandbox, _base_url, and _started_server but does not clear _redact_values. Stale secret values from the previous episode's env_vars remain in the set. This is safe (over-redaction is not a security problem) but semantically sloppy. Recommend clearing self._redact_values = set() in the finally block alongside the other state resets.

T1-4: tests/test_core/test_aca_provider_integration.py — test_aca_provider_real_sandbox_smoke is an async def with @pytest.mark.skipif(...). With asyncio_mode = "auto" this works correctly. No issue.

---

Tier 2: Alignment

ALIGNMENT FLAG: ACASandboxProvider is exported from the runtime package __init__.py but DaytonaProvider (the only comparable optional cloud provider) is deliberately not.

• Principle at stake: Consistency of the provider API surface; lazy-import hygiene (RFC 002 invariant 7 — installing core OpenEnv pulls in no cloud SDK).
• The concern: Adding ACASandboxProvider to __init__ creates an inconsistency with the established DaytonaProvider pattern (imported directly from its module). The azure SDK imports are correctly lazy, but the __init__.py re-export sets a precedent that differs from the optional-extra provider pattern.
• Suggested reviewer: @Darktex

ALIGNMENT FLAG: _DefaultACASandboxAdapter.__init__ instantiates the SandboxGroupClient synchronously in the ACASandboxProvider constructor when _adapter is None.

• Principle at stake: RFC 002 invariant 5 ("provider-local control plane, explicit cleanup") and the "fail fast with clear guidance" pattern.
• The concern: The SDK client is created eagerly in __init__, so credential/import errors fire at construction time (good for fast feedback), but a provider instance cannot be cheaply instantiated without Azure credentials. The _adapter injection escape hatch handles testing correctly. Worth confirming this construction-time credential model is intended before the pattern spreads.
• Suggested reviewer: @Darktex

---

Verdict

No Tier 1 blockers. The _redact_values not-cleared-on-stop issue (T1-3) is the closest to a genuine correctness concern, and it is minor (safe over-redaction, not a leak). The code is well-reasoned, security-conscious, and thoroughly tested. The two alignment flags are non-blocking design questions worth confirming before this pattern becomes the template for future cloud providers. Verdict: comment.

---

Automated review by Claude Code | Learn more

[Darktex]

Note: This is an automated review by Claude Code, not a human review.

---

Tier 1: Bugs & Correctness

Import ordering lint failure in __init__.py

File: src/openenv/core/containers/runtime/__init__.py

usort rejects the file as-is. The PR adds the aca_provider import after from .providers import ..., but alphabetically .aca_provider must come first:

# Current (PR) — fails `usort check`
from .providers import (ContainerProvider, ...)
from .aca_provider import ACASandboxProvider   # <-- wrong order
from .uv_provider import UVProvider

# Correct
from .aca_provider import ACASandboxProvider
from .providers import (ContainerProvider, ...)
from .uv_provider import UVProvider

usort check exits non-zero on this file; the pre-push hook will block this. Fix is a one-line reorder.

No other Tier-1 issues found

• ruff check and ruff format --check pass on all three new files (aca_provider.py, test_aca_provider.py, test_aca_provider_integration.py).
• All three ContainerProvider abstract methods (start_container, stop_container, wait_for_ready) are correctly implemented.
• requests is a declared core dependency; the wait_for_ready import is safe.
• The azure-containerapps-sandbox and azure-identity SDKs are gated behind the [aca] optional extra and imported lazily — no import-time cost for users who don't opt in. Correct.
• Cleanup on failed start is covered (stop_container() called in the except block of start_container).
• The _start_server double-shell-quoting pattern (shlex.quote(command) then bash -c {escaped}) is correct and is well-commented with the re-validation caveat.
• Redaction of injected secret values (_redact_values) is set before _sandbox is assigned and is applied before any server output is surfaced. Correct.
• The integration test (test_aca_provider_integration.py) is asyncio_mode = "auto" compatible and is gated behind OPENENV_ACA_INTEGRATION=1; it will not run in normal CI.

Tier 2: Alignment

ALIGNMENT FLAG: RFC amendment authored without original RFC author sign-off

• Principle at stake: "Design/Alignment (human-owned)" — PRINCIPLES.md and CONTRIBUTING.md state major architectural decisions require RFCs and that humans own the alignment phase. RFC 002 has four named authors; the amendment adding the "Cloud Sandbox Providers" section and six security invariants is authored solely by the PR author, with no amendment credit line and no indication of review by the original RFC authors. The new section defines invariants (S1–S6) that all future cloud providers must satisfy — these are binding architectural decisions, not implementation notes.
• The concern: Adding binding invariants to an existing RFC without review from that RFC's authors bypasses the alignment process. If the original authors disagree with (for example) "direct base URL, no client auth required" (invariant 1) or the egress defaults, the invariants could conflict with a future client-auth RFC that RFC 002 itself alluded to.

ALIGNMENT FLAG: ContainerProvider has no close() / context-manager protocol, but ACASandboxProvider requires it for deterministic cleanup

• Principle at stake: "Minimize lifecycle deltas" and consistent provider contract — the abstract contract shapes how all callers interact with providers.
• The concern: ACASandboxProvider adds close(), __enter__, and __exit__ that are not on ContainerProvider. The class docstring explicitly warns that a caller holding a bare ContainerProvider reference must keep the concrete type to release the client. This means any helper that accepts a ContainerProvider and calls start_container/stop_container will silently leak the SDK client, because stop_container does not close it — only close() does. DaytonaProvider has the same pattern. A default no-op close() on ContainerProvider (or a context-manager mixin) would let callers write with provider: without needing the concrete type.

---

Overall: well-structured, thoughtfully documented, strong test coverage. The one Tier-1 item (import order) is mechanical. The two alignment flags are for human decision, not blocking.

---

Automated review by Claude Code | Learn more

[thegovind]

Thanks for the second pass — addressed in 242617f.

Tier 1

Import ordering in __init__.py (usort). Fixed by removing ACASandboxProvider from __init__.py entirely (import + __all__) rather than reordering — which also resolves the alignment flag below. usort check now passes. It's imported from its module (from openenv.core.containers.runtime.aca_provider import ACASandboxProvider), matching DaytonaProvider. New tests: test_provider_imports_from_its_module and test_provider_not_in_runtime_package_all (asserts it's absent from both __all__ and the package namespace).

_redact_values not cleared on stop. Fixed — stop_container() now clears it in the finally and in the no-sandbox early-return (so a create_sandbox failure that recorded env vars is also covered). Tests added for both the active path and the create-failure path. (Verified no path reads _redact_values after stop: _server_died_message redaction runs inside wait_for_ready, which never calls stop_container.)

cmd in bash -c subshell. Added a comment making the trust boundary explicit: cmd is trusted orchestrator config, callers must not pass agent-controlled text, dynamic values must be pre-quoted (S5).

import requests module-level. Left as-is — it's a declared core dependency (this pass also confirmed it's safe), so the lazy-SDK-import pattern Daytona uses for the optional Azure SDK doesn't apply.

Tier 2 (alignment)

__init__.py export inconsistency with Daytona. Resolved by the removal above — ACA now follows the exact module-path import pattern as the only comparable optional cloud provider.

RFC amendment without original-author sign-off. Good call. I added a "Status: proposed amendment, pending review/sign-off by the RFC 002 authors" banner above the new subsection, and credited it in the Amendment History as proposed by me pending author review. @Darktex (and @pankit-eng / @jspisak / @zkwentz as RFC 002 authors) — could you confirm the S1–S6 invariants and the "direct base URL / no client-auth" stance? Happy to revise; I don't consider them finalized.

ContainerProvider.close() / context-manager not on the ABC. Still intentionally deferring the core ABC change to you, because the semantics are a real decision: close() defaulting to a no-op (ABC-compatible; providers with external clients override) vs defaulting to stop_container() (+ context manager on the ABC) behave differently for callers that distinguish "stop the container" from "release the provider client." I have both variants ready — tell me which you'd like and I'll add it in this PR (and drop ACA's local context manager in favor of the inherited one). Until then ACA stays safe via its own close()/with.

Also corrected stale RFC invariant numbers (S1–S6) in the provider/test comments — they referenced the pre-condensed S1–S10 scheme.

All 30 provider unit tests pass; tests/test_core is 165 passed / 11 skipped; ruff + usort clean.

[Darktex]

Note: This is an automated review by Claude Code, not a human review.

---

Alignment Review Report

Automated Checks

• Lint: PASS (the diff itself is clean — no ruff/usort violations detectable from the patch)
• Debug code: CLEAN — no debug artifacts in the new provider file

---

Tier 1: Fixes Required

1. src/openenv/core/containers/runtime/aca_provider.py (~line 678-693) — _redact_values populated before create_sandbox, but cleanup try starts after it

# secrets recorded here
self._redact_values = {value for value in (env_vars or {}).values() if value}

# create_sandbox is NOT inside the cleanup try/except
self._sandbox = self._adapter.create_sandbox(...)

try:                              # cleanup only covers this block
    if cmd:
        self._start_server(cmd)
    self._base_url = ...
except Exception:
    self.stop_container()
    raise

If create_sandbox raises (e.g. network timeout, quota error), _redact_values holds the injected secret values, self._sandbox is None, and stop_container() is never called automatically. The caller receives an exception with no indication they must call stop_container() to clear the retained secrets. The test test_failed_create_then_stop_clears_redact_values documents this gap explicitly but treats it as caller responsibility with no enforcement.

Fix: move the _redact_values assignment inside the cleanup try, or wrap the create_sandbox call in the same try/except, calling stop_container() on any exception (which safely handles self._sandbox is None).

2. src/openenv/core/containers/runtime/aca_provider.py — top-level import requests

All other providers in this module (DaytonaProvider, LocalDockerProvider, DockerSwarmProvider) import requests lazily inside the method that needs it. The new file imports it at the top level. requests is a core dependency so this is not a missing-import bug, but it is inconsistent with the module's lazy-import convention for optional/provider-specific dependencies. Move the import inside wait_for_ready.

---

Tier 2: Alignment Discussion

ALIGNMENT FLAG: RFC 002 amendment landing as normative without sign-off from original RFC authors

• Principle at stake: "Design/Alignment (human-owned): RFCs, principles, trade-off decisions" (CLAUDE.md); INVARIANTS.md "If a change would violate them, stop and flag for human review."
• The concern: rfcs/002-env-spec.md gains a substantial new section ("Cloud Sandbox Providers") with 7 protocol invariants and 6 security invariants. The amendment header in the diff itself says "proposed by @thegovind — pending review/sign-off by the RFC 002 authors." The original authors are @Darktex, @pankit-eng, @jspisak, @zkwentz. These invariants govern how all future cloud providers interact with the core protocol, which is squarely in the RFC authors' alignment jurisdiction. Landing them in the same PR as the first implementation means the invariants are de-facto ratified without the authors' explicit sign-off. The provider code is well-written and references these invariants throughout; the issue is solely the sequencing — the RFC amendment should be separately reviewed/approved before or alongside this PR, with the original authors explicitly signing off.
• Suggested reviewer: @Darktex

---

Summary

• 2 mechanical issues to fix (cleanup gap for _redact_values on create_sandbox failure; top-level import requests)
• 1 alignment point for human review (RFC 002 amendment landing without original-author sign-off)

The provider implementation itself is high-quality: the adapter pattern correctly isolates the preview SDK, the security invariants are actively enforced with tests, the egress warning is appropriate, and the opt-in integration test structure is correct. The RFC amendment gap is a process concern, not a correctness concern with the provider code itself.

---

Automated review by Claude Code | Learn more

[thegovind]

Thanks - addressed in df76847.

Tier 1

_redact_values cleanup on create_sandbox failure. Fixed with a two-stage cleanup in start_container:

• a create failure drops the recorded secret values and re-raises — it created nothing, so it deliberately does not call stop_container() (that would delete a sandbox this call didn't create, e.g. one left on a re-used provider);
• only a failure after the sandbox exists (_start_server/add_port) runs stop_container() to delete the just-created sandbox.

This auto-clears injected secrets on create failure (your ask) with no caller action and without the accidental-delete side effect. Tests: test_failed_create_auto_clears_redact_values (auto-clear + nothing deleted) and test_failed_restart_create_does_not_delete_existing_sandbox.

Top-level import requests. Moved into wait_for_ready (it's the only user), matching DaytonaProvider and the other providers in this module.

No regression to the validated demo. I replayed the live ACA demo's exact sequence — ACASandboxProvider(anonymous_port=True, cmd=..., egress_policy=deny_all_egress(...)) → start_container("python-3.11", port=8000) → wait_for_ready → stop_container — against the patched code; it passes unchanged (the disk name python-3.11 is a bare public disk, not matched by the OCI guard). Added it as a parametrized regression test.

Tier 2 — RFC 002 amendment sign-off

Agreed this is the authors' call, and I don't consider the invariants finalized. I've marked the new "Cloud Sandbox Providers" section with a "Status: proposed amendment, pending review/sign-off by the RFC 002 authors" banner + an Amendment-History credit.

@Darktex @pankit-eng @jspisak @zkwentz (RFC 002 authors) and @burtenshaw - could you review/sign off on the protocol + security invariants (esp. invariant 1 "direct base URL / no client-auth" and the egress defaults)? Happy to split the RFC amendment into its own PR for separate review and rebase this implementation on top if you'd prefer that sequencing - just let me know.

(The ContainerProvider.close()/context-manager-on-the-ABC question from the prior pass still stands - I have both variants ready, no-op default vs stop_container() default; will add whichever you choose.)

All 32 provider unit tests pass; tests/test_core is 167 passed / 11 skipped; ruff + usort clean.

[Darktex]

Note: This is an automated review by Claude Code, not a human review.

---

Alignment Review Report

Automated Checks

• Lint: SKIP - The new files (aca_provider.py, test_aca_provider.py) are not in the working tree (PR not merged); static review of the diff text shows no obvious ruff/usort violations and the PR reports ruff clean.
• Debug code: CLEAN - No debug artifacts (print, breakpoint, pdb) in the new provider or test files.

---

Tier 1: Fixes Required

• rfcs/002-env-spec.md — The top-level **Amended**: June 14, 2026 header is advanced even though the amendment itself is labelled > **Status:** proposed amendment, pending review/sign-off by the RFC 002 authors. These signals contradict each other. The Amended date should advance only when the RFC authors sign off; until then keep the prior date or label it **Amended (proposed)** to avoid misrepresenting the document's authoritative state.

• src/openenv/core/containers/runtime/aca_provider.py — Double-start sandbox leak. start_container unconditionally overwrites self._sandbox without first cleaning up an already-active sandbox. If a caller calls start_container twice on the same provider instance without an intervening stop_container, the old sandbox is orphaned — never deleted, running up cost indefinitely. The create-failure path is handled (test_failed_restart_create_does_not_delete_existing_sandbox), but the successful-double-start path is not. Fix: raise RuntimeError if self._sandbox is not None at the top of start_container (or call stop_container() first), and add a test for both behaviors.

• pyproject.toml — The aca extra pins azure-containerapps-sandbox>=0.1.0b2 with only a lower bound on a preview (b2) SDK. The provider docstring says to pin this SDK and re-validate after upgrades; the dependency declaration should reflect that with an upper bound (e.g. >=0.1.0b2,<0.2), matching how other volatile SDKs are handled.

---

Tier 2: Alignment Discussion

ALIGNMENT FLAG: RFC amendment merged without original author sign-off

• Principle at stake: Design/Alignment is human-owned (CLAUDE.md workflow); RFC 002 is co-authored by @Darktex, @pankit-eng, @jspisak, @zkwentz and its status is "In Review".
• The concern: The PR adds a substantive new section ("Cloud Sandbox Providers") with 7 protocol invariants and 6 security invariants to RFC 002, explicitly noting it is "proposed amendment, pending review/sign-off." Merging would land un-signed-off RFC text alongside production code that already cross-references those invariants as normative (# RFC 002 security invariant S1, etc.). If the authors later disagree, the code docstrings would cite non-existent agreed-upon invariants. Either get sign-off from an RFC author before merging, or keep the RFC change in a clearly "draft" document and have aca_provider.py docstrings cite "draft RFC 002 amendment".
• Suggested reviewer: @Darktex

ALIGNMENT FLAG: close() / context manager not on ContainerProvider base

• Principle at stake: Consistent interface across all providers (RFC 002); PATTERNS.md provider contract.
• The concern: ACASandboxProvider adds close(), __enter__, __exit__ which are not part of the ContainerProvider ABC (and DaytonaProvider lacks them too, leaking its SDK client today). This creates a two-tier provider contract that grows harder to manage as more cloud providers are added. The more durable fix is to add close() (default no-op) to ContainerProvider so the full lifecycle — including SDK client release — is part of the contract. Design discussion, not a blocking bug, but the right time to decide since this is the second cloud provider.
• Suggested reviewer: @Darktex

---

Summary

• 3 mechanical issues to fix (RFC date inconsistency, double-start sandbox leak, preview SDK upper-bound pin)
• 2 alignment points for human review (RFC sign-off gate, close() on base class)

---

Automated review by Claude Code | Learn more

[thegovind]

Thanks @Darktex — addressed in 648d8e1. I rebased onto the latest branch (main was merged in) first.

Tier 1 (all fixed)

RFC Amended date vs "proposed" status. Reverted the top-level **Amended**: back to the last signed-off date (November 12, 2025) with a parenthetical pointing to the proposed amendment in the history — so the header no longer implies the new section is ratified.

Double-start sandbox leak. start_container now raises RuntimeError if a sandbox is already active (a second start would orphan the running one). Chose raise over auto-stop so a live episode is never silently killed. Tests: test_double_start_raises_and_preserves_existing_sandbox (asserts no second create_sandbox, existing sandbox + base_url intact), test_start_after_stop_is_allowed.

Preview SDK upper bound. azure-containerapps-sandbox>=0.1.0b2,<0.2.0. Scoped the cap to the preview SDK; left GA azure-identity lower-bounded to match the repo's lower-bound-everywhere convention.

Tier 2 → resolved, not just deferred

close() / context manager on the base contract. Done — added close() (default no-op, per your recommendation) + __enter__/__exit__ to ContainerProvider, consistent with the sibling RuntimeProvider which already has a context manager. __enter__ is generic so subclass typing is preserved. ACASandboxProvider now inherits the context manager and keeps its close() override (stop + SDK-client release), so a caller holding a bare ContainerProvider can release resources polymorphically. Backwards-compatible — the other providers inherit a no-op close(); full suite: 1284 passed, 0 failures.

RFC amendment sign-off. Per your accepted option (b): the amendment is clearly marked proposed/pending sign-off (banner + history), the Amended date is no longer advanced, and the provider's module + class docstrings now state the RFC 002 S<n> references are to the proposed amendment. So nothing in this PR asserts un-signed-off invariants as ratified.

That leaves the one thing only you all can do — sign-off: @Darktex @pankit-eng @jspisak @zkwentz (RFC 002 authors) and @burtenshaw, could you review the proposed "Cloud Sandbox Providers" invariants? I'm equally happy to split the RFC into its own PR and rebase this implementation on top — your call; one word and I'll do it.

No regression to the validated demo

The live ACA demo does a single start_container("python-3.11", port=8000) on a fresh provider; I replayed its exact sequence post-patch and it passes (the disk python-3.11 is a bare public disk, untouched by the OCI guard; the double-start guard only affects re-use, which the demo never does).

Provider unit tests: 34 passed; tests/test_core: 169 passed / 11 skipped; full suite 1284 passed; ruff + usort clean.

[burtenshaw]

• No new blockers from the latest patch.
• main positives: thin, provider-neutral contracts; no vendor-specific core protocol; lifecycle/security stay orchestration-only.
• I am comfortable keeping the RFC amendment in this PR if RFC 002 authors sign off here; split only if the invariants need broader TC debate.
• Small wording nit before sign-off: soften "Because every provider has these" to "Cloud sandbox providers generally expose these" so the RFC does not overclaim neutrality.

[thegovind]

Thanks @burtenshaw. Addressed the wording nit in 663357c: softened "Because every provider has these" to "Cloud sandbox providers generally expose these, so ..." in the RFC 002 amendment, so it no longer overclaims universal neutrality.

Agreed on the rest. The contracts stay thin and provider-neutral, there is no vendor-specific core protocol, and lifecycle plus security stay orchestration-only. I am happy to keep the RFC amendment in this PR pending sign-off from the RFC 002 authors (@Darktex, @pankit-eng, @jspisak, @zkwentz), and glad to split it into its own PR if the invariants need broader TC debate.

[Darktex]

Note: This is an automated review by Claude Code, not a human review.

---

Alignment Review: feat: add Azure Container Apps cloud sandbox provider

The implementation is well-structured, security-conscious, and consistent with DaytonaProvider conventions. The tests are thorough (24 unit tests + adapter shape verification + opt-in integration test). The concerns below are a mix of fixable correctness issues and process-level alignment questions.

---

Tier 1: Fixes Required

• aca_provider.py:730–732 — Exception masking during cleanup (high diagnostic risk). In start_container, except Exception: self.stop_container(); raise will replace the original exception with any RuntimeError from delete_sandbox (e.g. a network failure during cleanup). The caller loses the root-cause error entirely. Fix: wrap the stop_container() call in a try/except and suppress or chain the secondary exception so the original propagates. (Note: DaytonaProvider has the same issue at daytona_provider.py:490–491 — worth fixing both.)

• aca_provider.py:882–892 — adapter.close() called on double-close. close() calls stop_container() (idempotent: clears _sandbox) and then unconditionally calls self._adapter.close() if _owns_adapter. Since _owns_adapter is never cleared, a second close() (explicit call + context-manager exit, or two context-manager exits) calls adapter.close() again. For a preview SDK, non-idempotent close is plausible. Fix: set self._owns_adapter = False after calling self._adapter.close().

• providers.py:941 — __exit__ return type should be Optional[bool], not None. Returning None is semantically correct (does not suppress), but the annotation is wrong per the protocol. mypy will flag callers. Fix: -> Optional[bool].

• aca_provider.py:447–453 — Whitespace-only disk name passes _source_from_image guard. "disk: " (space after colon) produces disk = " ", which is truthy, so it passes the if not disk check and reaches the ACA SDK as " ". Every other bad input gets a clear ValueError. Fix: add .strip() to extracted values and check not disk.strip().

---

Tier 2: Alignment Discussion

ALIGNMENT FLAG: Implementation lands before RFC 002 amendment is ratified.

• Principle at stake: CLAUDE.md and PRINCIPLES.md: design/alignment phase is human-owned; RFCs document decisions that must not change without a new RFC.
• The concern: The "Cloud Sandbox Providers" amendment added to rfcs/002-env-spec.md in this PR is explicitly marked "pending review/sign-off by the RFC 002 authors" (@Darktex, @pankit-eng, @jspisak, @zkwentz). The implementation (aca_provider.py) and tests land in the same PR against those proposed invariants. If the RFC authors want to revise any invariant (e.g. the HTTPS-only enforcement, the "direct base URL" requirement, or the auto-suspend defaults), the code, tests, and docs already embed those choices. The usual flow is RFC sign-off first, then implementation.
• Suggested reviewer: @Darktex

ALIGNMENT FLAG: ContainerProvider.start_container() docstring contract broadened before RFC ratification.

• Principle at stake: INVARIANTS.md: Gymnasium API signatures must not change without a major version bump; shared interface contracts should be stable.
• The concern: providers.py changes the image parameter docstring from "Container image name (e.g., echo-env:latest)" to "Provider-specific container source identifier." This is a semantic loosening of the shared ContainerProvider abstraction that all providers and callers rely on. It's tied to the pending RFC amendment — if that amendment is not ratified as-written, the base class docstring has already drifted. Should be gated on RFC sign-off.
• Suggested reviewer: @Darktex

ALIGNMENT FLAG: deny_all_egress() static helper naming as a precedent question.

• Principle at stake: RFC 002 (proposed amendment): "such helpers stay provider-local until multiple providers demonstrate the same semantics — OpenEnv does not standardize a cloud-only operation prematurely."
• The concern: The method is correctly placed on ACASandboxProvider (not promoted to ContainerProvider), but it returns an ACA-SDK EgressPolicy object and is named generically enough that a future provider adding a method with the same name but different semantics could create interface confusion. Low severity — currently provider-local as intended — but worth noting in the RFC discussion so the naming convention is deliberate.
• Suggested reviewer: @Darktex

---

Automated review by Claude Code | Learn more

[thegovind]

Addressed the Tier 1 items from the latest automated pass in 415a307:

• start_container cleanup no longer masks the original error: stop_container() is wrapped so a delete_sandbox failure during cleanup cannot replace the root cause.
• close() sets _owns_adapter = False after closing, so a second close() or context exit will not double-close a non-idempotent preview client.
• ContainerProvider.__exit__ is now typed Optional[bool].
• _source_from_image strips extracted values and rejects whitespace-only sources like disk: and disk-id: with the same clear error.

Added 3 tests (36 total, all passing; ruff and usort clean): cleanup does not mask the original error, double close closes the adapter once, and whitespace source rejection.

The Tier 2 items are the same RFC 002 sign-off process point you already covered. The amendment stays in this PR, clearly marked "pending review/sign-off by the RFC 002 authors", and the base docstring generalization is gated on that. No design change, only the wording you flagged.

@burtenshaw, candidly this can iterate forever. Each automated pass surfaces a fresh set of style and process nits with no human decision, so there is no natural point of closure. You flagged no blockers and signed off on the approach. If you are comfortable, could you approve (and merge, or ping an RFC 002 author for the sign-off) so this can land? I have several follow-up PRs queued behind it and would rather stop re-spinning this one. Happy to split the RFC amendment into its own PR if that is the faster path to a decision.

[bot-ci-comment]

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.