fix(engine): gate the experimental cloud engine behind an opt-in (#966)

[frankbria]

Closes #966.

E2B cloud execution is out of launch scope and does not work end to end, but it was listed next to the engines that do and was reachable with a plain --engine cloud. Per the issue, this gates it rather than fixing it — the five PARKED defects are deliberately untouched and recorded as the checklist for lifting the gate.

The gate

CODEFRAME_ENABLE_CLOUD_ENGINE=1 unlocks it. Closed by default:

$ cf work start <id> --execute --engine cloud
Error: The 'cloud' engine (E2B) is EXPERIMENTAL and unsupported. It is not
part of the supported surface and has known defects that can leave your
working tree inconsistent with what the gates then validate. To try it
anyway, set CODEFRAME_ENABLE_CLOUD_ENGINE=1. Supported engines: built-in,
claude-code, codex, kilocode, opencode, plan, react.

Both resolve_engine and get_external_adapter refuse it, so CODEFRAME_ENGINE=cloud is not a way around the flag. The message names the variable — a gate, not a dead end. cloud stays in VALID_ENGINES so the refusal is specific rather than a generic "unknown engine"; the new ADVERTISED_ENGINES set drives what gets offered.

The gate is also checked before any run record is created. resolve_engine runs inside execute_agent, which the CLI reaches after start_task_run — refusing there alone stranded an IN_PROGRESS run, the exact failure the neighbouring "validate API key before creating run record" block exists to prevent. Both the single-run and batch CLI paths now resolve the engine in that pre-run block. A test pins it by asserting get_latest_run(...) is None.

Off the advertised surface

Five places, not one:

Surface	Before	After
--engine help (×2)	listed cloud	advertised set only
worked examples in help	--engine cloud --cloud-timeout 45	removed
cf engines list	iterated VALID_ENGINES	ADVERTISED_ENGINES; opted-in it appears as external, EXPERIMENTAL
EnvironmentConfig.validate	suggested cloud for a typo'd engine	advertised set only
CLAUDE.md / PRODUCT_ROADMAP.md	counted under "all complete"	"EXPERIMENTAL and gated"

IsolationLevel.CLOUD is a separate thing and stays unimplemented; its error no longer points at --engine cloud as a supported route.

Known-limitations note (AC3)

CLAUDE.md → CODEFRAME_ENABLE_CLOUD_ENGINE records the five parked defects as a numbered lift-the-gate checklist: the pip install codeframe / codeframe-ai package-name bug, unhandled CommandExitException, broken sync-back, unbatched upload, and non-autospec mocks that make every failure branch dead code under a green suite. A test asserts the note names them, so it cannot silently drift.

Verification

• 35 new tests in tests/core/test_cloud_engine_gate_966.py — closed by default (incl. the env-var route and get_adapter/get_external_adapter defence in depth), open with the opt-in (5 truthy / 5 falsy values), absent from each advertised surface, no run record on refusal.
• Mutation-checked: neutering _reject_cloud_unless_enabled fails 9 tests; removing the CLI pre-run check fails the dangling-run test with a live RUNNING record.
• Full gate green: pytest tests/ --ignore=tests/e2e -m "not lifecycle" → 6085 passed, 49 skipped, 0 failed (16m07s), run with env -u DATABASE_PATH -u CODEFRAME_AUTH_REQUIRED -u ANTHROPIC_API_KEY -u AUTH_SECRET -u OPENAI_API_KEY to mirror CI's ambient environment. ruff check clean.
• Third-party review: codex review --base main raised one [P2] — the execution path was gated but cf engines list and the config validator still offered it. Verified against the running CLI (cloud was visible in the table) and fixed in 6dba19f.

Three pre-existing tests were adjusted rather than weakened: two E2B registry tests and test_all_valid_engines_resolve now set the opt-in, so they still cover the cloud path instead of dropping it.

Known limitations

• The cloud engine still does not work. That is the point — this PR makes it unreachable, not correct. All five PARKED criteria remain open by design.
• check_requirements("cloud") is still callable directly. It reports env-var presence and creates nothing, and cf engines list no longer surfaces it; gating a pure predicate would break the opted-in list rendering for no safety gain.

[github-actions]

Precision bug-hunting review in progress

Reviewing PR #1099 (gate the experimental cloud engine) for concrete defects only — logic errors, crash paths, security, data loss, contract breaks. Style/architecture/coverage are out of scope (covered elsewhere).

• Gather PR diff and existing comments
• Read engine_registry.py, config.py, cli/app.py and surrounding code
• Trace callers/callees to confirm each suspicion
• Post inline findings (confirmed defects with failure scenarios)
• Post summary table

View job run

[claude]

Review

Reviewed the diff (gh pr diff 1099) plus the surrounding code in engine_registry.py, cli/app.py, cli/engines_commands.py, core/config.py, core/sandbox/context.py, and the v2 tasks router. This is the first substantive review on the PR.

Summary

Solid, well-scoped gate. The design does what the description claims:

• Defense in depth is real. _reject_cloud_unless_enabled is checked in both resolve_engine (the CLI/env path) and get_external_adapter (the direct-lookup path used by get_adapter, which runtime calls). I traced every construction site of E2BAgentAdapter — there's exactly one, inside the gated get_external_adapter branch — so there's no way to instantiate the adapter that skips the check.
• The dangling-run fix is correctly placed. Moving resolve_engine(engine) into the existing pre-run validation block (before start_task_run/batch creation, for both work_start and batch_run) genuinely prevents the "gate raises inside execute_agent, after the run record already exists" failure mode described in the PR body. test_work_start_execute_creates_no_run pins this well.
• The off-advertised-surface changes check out independently. I verified --engine help text, the removed worked example, cf engines list (now ADVERTISED_ENGINES, with an EXPERIMENTAL label when opted in), the config validator's suggestion list, and the IsolationLevel.CLOUD error message — all five match what the PR claims.
• No equivalent gap on the web/API side. I checked whether POST /api/v2/tasks/execute and /approve (which persist a batch before execution, same class of bug) could reach engine="cloud" — they can't: StartExecutionRequest/ApproveTasksRequest already restrict engine to Literal["plan", "react"]/("plan", "react") via pydantic validators, unrelated to this PR. So there's nothing to fix there.
• check_requirements("cloud") staying ungated is a reasonable, documented call — it only imports the adapter class and calls a classmethod, never constructs it, so leaving it out of the gate doesn't reopen the hole.
• Test coverage is thorough for both halves of the gate (closed-by-default, opt-in truthy/falsy parsing, absent-from-discovery-surfaces) and the regression it fixes.

Minor, non-blocking observations

1. batch_run --dry-run --engine cloud skips the new check. In codeframe/cli/app.py, the batch_run dry-run branch (if dry_run: ... return) returns before the new resolve_engine(engine) pre-check, whereas work_start's equivalent check runs unconditionally regardless of --dry-run. Not a dangling-run bug (dry run creates nothing), just an inconsistency — a batch dry-run preview with --engine cloud won't surface the "experimental, gated" message the way a real run or a single-task dry-run would.
2. No CHANGELOG entry. CHANGELOG.md's [Unreleased] section documents other "off by default, opt-in via env var" behavior changes (e.g. WORKSPACE_ROOT, bootstrap registration) with a similar writeup under Security/Changed. This PR changes existing behavior for anyone currently running --engine cloud without a new env var, so it seems like a fit for that section, even if it's not security-critical.

Neither point blocks merge — the core gating logic, its placement, and the test coverage are all solid.

[frankbria]

Third-party review — codex review --base main

Run twice: pre-PR on the branch diff, and post-PR on the final diff. Both findings were verified against the running CLI before fixing.

Pre-PR — [P2] Hide cloud from engine discovery paths (engine_registry.py)

cf engines list still iterates VALID_ENGINES, so the experimental cloud engine remains visible even without CODEFRAME_ENABLE_CLOUD_ENGINE.

Verified — cf engines list printed cloud | external | ✓ E2B_API_KEY alongside the working engines, and EnvironmentConfig.validate offered it as a fix for a typo'd engine:. Fixed in 6dba19f.

Post-PR — [P3] Use advertised engines in requirement errors (engine_registry.py)

check_requirements() still builds its validation message from VALID_ENGINES, so cloud is presented as valid.

Verified — cf engines check nope listed cloud. This was the fourth such list, and it survived precisely because the previous fix patched three sites by hand. Fixed in b63cdff by introducing suggestable_engines() as the single source for every "must be one of" message, with a test asserting they agree. That also exposed an asymmetry worth noting: resolve_engine's invalid-engine message omitted cloud even when the caller had opted in.

No Critical or Major findings. Both accepted and fixed; neither was dismissed.

[github-actions]

Claude finished @frankbria's task in 4m 55s —— View job

---

Precision bug-hunting review — ✅ no defects found

Reviewed the full diff plus surrounding code in engine_registry.py, cli/app.py, cli/engines_commands.py, core/config.py, core/sandbox/context.py, core/runtime.py, core/conductor.py, adapters/e2b/adapter.py, and ui/routers/tasks_v2.py. Scope: concrete defects only (logic errors, crashes, security, data loss, contract breaks).

No new concrete defects introduced by this PR. The gate is correct and the defense-in-depth is real:

• Single construction site, both doors locked. E2BAgentAdapter(...) is constructed in exactly one place — engine_registry.py:169, inside get_external_adapter. _reject_cloud_unless_enabled is checked in both resolve_engine (the CLI/env route) and get_external_adapter (the direct route get_adapter/runtime use). _reject_cloud_unless_enabled logic is sound: it raises only when engine == "cloud" and not cloud_engine_enabled().
• No API-side gap. StartExecutionRequest/ApproveTasksRequest validate engine against ("plan", "react") and the GET execution endpoints use Literal["plan", "react"] (tasks_v2.py:63,120,913,1034) — cloud cannot reach the conductor or runtime via REST, independent of this PR.
• Dangling-run fix is correctly placed. The pre-run resolve_engine(engine) sits inside if execute: in both work_start and batch_run — the only path where the gate would otherwise raise inside execute_agent after start_task_run. The batch worker subprocess (conductor.py:2571) re-enters cf work start … --execute, so it re-runs the same pre-run check; cf work batch run also pre-checks before conductor.start_batch. No path leaves a RUNNING record for a refused engine.
• CODEFRAME_ENGINE=cloud is not a bypass — resolve_engine reads it, then hits the gate (test_the_env_var_route_is_gated_too).
• Advertised-surface consolidation is complete. All four user-facing "must be one of" lists route through suggestable_engines() (resolve_engine, check_requirements, config.validate, engines list); test_every_suggestion_list_agrees pins them. cf engines list opts cloud in with an EXPERIMENTAL label, opts it out entirely otherwise.
• check_requirements("cloud") staying ungated is safe — it imports the class and calls a classmethod, never constructs the adapter, and engines list no longer drives it when opted out.

Already covered by prior review (not re-posted): the batch_run --dry-run --engine cloud inconsistency (dry-run returns before the pre-check; creates nothing, so no defect) and the missing CHANGELOG entry. Both non-blocking; the latest commits (6dba19f, b63cdff) address the discovery-surface findings raised by codex review but not these two, which is consistent with them being stylistic/UX rather than defects.

Checklist:

• Gather PR diff and existing comments
• Read engine_registry.py, cli/app.py, cli/engines_commands.py, config.py, sandbox/context.py and surrounding code
• Trace callers/callees to confirm each suspicion
• Post inline findings (none — no confirmed defects)
• Post summary table

[claude]

Review (commit b63cdff, latest push)

Re-reviewed the full diff including the two codex review follow-up commits (6dba19f, b63cdff) on top of what was covered in my earlier review at 05:14. Confirming consistency with that review rather than repeating it in full.

What I re-verified

• suggestable_engines() is genuinely the single source now. All four "must be one of" sites (resolve_engine, check_requirements, EnvironmentConfig.validate, cf engines list) route through it, and it correctly reflects cloud_engine_enabled() read at call time. _reject_cloud_unless_enabled is still called from both resolve_engine and get_external_adapter, and get_adapter/runtime.execute_agent reach the gate through those — no new bypass introduced by the P3 commit.
• EnvironmentConfig.validate() correctly keeps cloud in the membership check (self.engine not in VALID_ENGINES) while only excluding it from the suggestion list — so a workspace config with engine: cloud still passes config validation (as it should, since it's a real gated engine, not a typo) and gets refused later at resolve_engine/get_external_adapter. That asymmetry is intentional and correctly implemented.
• No other tests reference cloud in cf engines list output that this change would silently break — confirmed via grep, consistent with the PR's reported full green suite.
• Traced _get_adapter_class("cloud") (used by check_requirements) — it only imports and returns the class, never instantiates, so check_requirements("cloud") staying ungated is safe as documented.

Still outstanding (non-blocking, carried over from the earlier review)

• batch_run --dry-run --engine cloud still returns before the new resolve_engine(engine) check (codeframe/cli/app.py ~line 4203 vs. 4211), so a batch dry-run preview with the cloud engine won't show the gate message the way work_start's dry-run does. Not a correctness bug (nothing is created), just an inconsistency between the two dry-run paths.
• Still no CHANGELOG.md [Unreleased] entry, despite this changing observable behavior for anyone currently running --engine cloud without the new env var (other opt-in-by-env-var changes like WORKSPACE_ROOT do get an entry there).

No new defects found in the latest commits. The gate placement, defense-in-depth (resolve_engine + get_external_adapter both checked), and the dangling-run fix all hold up under this pass.

[frankbria]

Demo — acceptance criteria → outcome evidence

AC1 — rejected unless opted in; message says experimental and unsupported

$ cf work start <id> --execute --engine cloud
Error: The 'cloud' engine (E2B) is EXPERIMENTAL and unsupported. It is not part
of the supported surface and has known defects that can leave your working tree
inconsistent with what the gates then validate. To try it anyway, set
CODEFRAME_ENABLE_CLOUD_ENGINE=1. Supported engines: built-in, claude-code,
codex, kilocode, opencode, plan, react.

Outcome, not just the message: runtime.get_latest_run(ws, task.id) is None — no run record was created. Removing the pre-run check makes that assertion fail with a live RUNNING record, so it is testing state, not text.

CODEFRAME_ENGINE=cloud (the env route) produces the same refusal — not a way around the flag.

AC2 — reachable with the opt-in

$ CODEFRAME_ENABLE_CLOUD_ENGINE=1 python -c "..."
resolve_engine("cloud") -> cloud

And the real adapter is constructed, not stubbed: get_external_adapter("cloud", timeout_minutes=45) returns an E2BAgentAdapter with _timeout_minutes == 45. Truthy values 1/true/TRUE/yes/on open it; 0/false/no/off/"" keep it shut.

AC2 (cont.) — off every advertised list

$ cf engines list                      # cloud absent
built-in | claude-code | codex | kilocode | opencode | plan | react

$ CODEFRAME_ENABLE_CLOUD_ENGINE=1 cf engines list
cloud    | external, EXPERIMENTAL | ✓ E2B_API_KEY      # present AND labelled

$ cf engines check nope
Error: Invalid engine 'nope'. Must be one of: built-in, claude-code, codex,
kilocode, opencode, plan, react

Same for --engine help, the worked examples, EnvironmentConfig.validate, and the two docs that counted it under "all complete".

AC3 — known-limitations checklist

CLAUDE.md → CODEFRAME_ENABLE_CLOUD_ENGINE, five numbered defects. A test asserts the note names codeframe-ai, CommandExitException and sync-back, so it cannot drift into a vague "experimental".

AC4 — tests

36 tests in tests/core/test_cloud_engine_gate_966.py. Mutation-checked twice: neutering the gate fails 9; removing the CLI pre-run check fails the dangling-run test.

---

Deliberate deviation on --isolation cloud

AC1 asks for a rejection message saying "experimental and unsupported". At the CLI it is not a choice at all:

$ cf work start <id> --execute --isolation cloud
Invalid value for '--isolation': 'cloud' is not one of 'none', 'worktree'.

I left this as-is. IsolationLevel.CLOUD is unimplemented and stays unimplemented — it is not gated-but-available like the engine, so there is no opt-in that would make it work, and adding cloud back to the accepted choices just to print a nicer refusal would re-list it in --help. The programmatic path does carry the wording: validate_isolation raises NotImplementedError("IsolationLevel.CLOUD is not implemented ... EXPERIMENTAL, unsupported E2B engine ..."), and that message no longer points at --engine cloud as a supported route.

Flagging it rather than claiming the AC verbatim.

[frankbria]

Both carried-over items from the review are now fixed in fdd0f5a — neither was blocking, but both were real.

batch_run --dry-run --engine cloud skipped the gate. Confirmed: the dry-run early return sat at app.py:4203, ahead of the check at 4211, so a batch dry-run printed a "here is what will run" preview for an engine that would then be refused. Worth fixing on its own terms — the point of a dry-run is to tell you what will happen. The check now sits ahead of the return, where it also doubles as the pre-run guard for conductor.start_batch. Pinned by test_batch_dry_run_refuses_too (exit 1 + the experimental message), which was RED before the move.

No CHANGELOG entry. Correct, and it matters more than usual here: this breaks existing --engine cloud invocations. Added under [Unreleased] → Changed, stating the break and the one-line upgrade (set the env var), matching how WORKSPACE_ROOT is documented.

Re-running the full backend gate on the final commit.

[github-actions]

Claude finished @frankbria's task in 4m 33s —— View job

---

Precision bug-hunting review — ✅ no defects found

Reviewed the full diff (including the latest push: batch dry-run gate + CHANGELOG) plus surrounding code in engine_registry.py, cli/app.py, cli/engines_commands.py, core/config.py, core/runtime.py, core/conductor.py, core/sandbox/context.py, adapters/e2b/adapter.py. Scope: concrete defects only (logic errors, crashes, security, data loss, contract breaks).

No concrete defects introduced by this PR — including the latest commit. The gate is correct and defense-in-depth holds on every path I traced:

• Single construction site, both doors locked. E2BAgentAdapter(...) is built in exactly one place — engine_registry.py:169, inside get_external_adapter. _reject_cloud_unless_enabled runs in both resolve_engine (:121) and get_external_adapter (:145); get_adapter / runtime.execute_agent:889 reach the adapter only through the latter, and execute_agent:759 re-resolves via resolve_engine. The predicate is correct: raises only on engine == "cloud" and not cloud_engine_enabled().
• Env parsing is correct. cloud_engine_enabled() does .strip().lower() in {"1","true","yes","on"} — unset→""→falsy, and "0"/"false" are correctly falsy (no bool("0") trap).
• All three engine sources are caught. The engine var is resolved from --engine → CODEFRAME_ENGINE → .codeframe/config.yaml engine: (app.py:2692/:4119) before the pre-check, so a config-supplied engine: cloud hits the gate too — not just the flag.
• Dangling-run fix is correctly placed on both paths. work_start checks before runtime.start_task_run (:2762); batch_run checks before the dry-run return and conductor.start_batch (:4249). The batch worker re-enters cf work start --execute (conductor.py:2571), re-running the same check — no path leaves a RUNNING record for a refused engine.
• work_resume (the other --engine command) is safe. Its gate ValueError is caught at app.py:2903, the run is fail_run'd, and it exits 1 — no traceback, no dangling-RUNNING wedge, no bypass. (No new pre-check needed there since resume_run reuses an existing run rather than creating one.)
• CODEFRAME_ENABLE_CLOUD_ENGINE=cloud-via-config and the env route are not bypasses — both fold into the resolved engine and hit the gate.
• API surface needs no fix. StartExecutionRequest/ApproveTasksRequest already restrict engine to plan/react (independent of this PR).
• check_requirements("cloud") staying ungated is safe — _get_adapter_class imports the class and calls a classmethod, never constructs; engines list no longer drives it when opted out.
• No fifth suggestion list. All four "must be one of" sites route through suggestable_engines(); membership checks correctly stay on VALID_ENGINES (so a gated engine: cloud still passes config validation and is refused later).

Items the prior reviews flagged are resolved in the latest commits (batch --dry-run --engine cloud now refuses; CHANGELOG [Unreleased] → Changed entry added) — I re-verified both, and neither introduced a defect.

Checklist:

• Gather PR diff and prior comments (don't repeat findings)
• Read engine_registry.py, cli/app.py, cli/engines_commands.py, core/config.py, sandbox/context.py, runtime.py, conductor.py and surrounding callers
• Trace callers/callees to confirm each suspicion
• Post inline findings (none — no confirmed defects)
• Post summary table

[claude]

Review (final commit 00a5178)

Re-checked the final state after the batch dry-run fix and CHANGELOG addition, on top of my two earlier passes (05:14, 05:17) and the independent findings from codex review and the automated bug-hunter. Not repeating what's already confirmed there — just what I specifically re-verified this pass.

• Both carried-over items are actually fixed. batch_run's resolve_engine(engine) check now sits ahead of the if dry_run: ... return (app.py:4206-4212, before the dry-run block at 4214), so a batch dry-run with --engine cloud now errors out instead of printing a misleading preview. CHANGELOG.md has the [Unreleased] → Changed entry stating the break and the one-line fix (set the env var).
• The IsolationLevel.CLOUD message change doesn't break the pre-existing test_dead_surface_955.py::TestCloudIsolationMessage::test_it_points_at_the_engine_that_exists. That test asserts "--engine cloud" in message; the new wording (`--engine cloud`, gated behind CODEFRAME_ENABLE_CLOUD_ENGINE=1`) still contains that literal substring, and doesn't reintroduce the word "future" the test also checks for. Confirmed by reading the message text directly rather than assuming.
• Traced runtime.execute_agent (the path outside the CLI's pre-run check) — it calls resolve_engine then get_external_adapter, both gated via _reject_cloud_unless_enabled, consistent with what the automated bug-hunter already reported for the batch-subprocess re-entry path.
• No other stray --engine cloud references left in non-test, non-doc source (grep across codeframe/); the two remaining hits outside this PR's own files (e2b/adapter.py's install-hint string, docs/archive/) are pre-existing/archived and out of scope.

No new defects. This is consistent with the prior reviews: the gate is correctly placed with real defense-in-depth, the dangling-run fix is sound, and the two non-blocking follow-ups from earlier passes are now closed out.